Change in vdsm[master]: tox: added disable tox style validation option to Makefile

Irit Goihman has uploaded a new change for review.

Change subject: tox: added disable tox style validation option to Makefile
......................................................................

tox: added disable tox style validation option to Makefile

This patch adds DISABLE_STYLE_VALIDATION which is an environment
variable that disables tox run.
This option is useful for developers who encounter issues running tox
on their environment and want to skip tox style validation.
use it by the following:
make check DISABLE_STYLE_VALIDATION=1

Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Signed-off-by: Irit Goihman <igoihman(a)redhat.com&gt;
---
M Makefile.am
1 file changed, 15 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/17/59417/1

diff --git a/Makefile.am b/Makefile.am
index 9ac2515..9d43d58 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -81,18 +81,26 @@
 
 .PHONY: pyflakes
 pyflakes:
-	if [ -n "$(TOX)" ]; then \
-		tox -- pyflakes; \
+	if [ "$(DISABLE_STYLE_VALIDATION)" == "1" ]; then \
+		echo "Warning: style validation disabled, skipping check"; \
 	else \
-		echo "Warning: tox is not installed, skipping check"; \
-	fi
+		if [ -n "$(TOX)" ]; then \
+			tox -- pyflakes; \
+		else \
+			echo "Warning: tox is not installed, skipping check"; \
+		fi \
+	fi 
 
 .PHONY: pep8
 pep8:
-	if [ -n "$(TOX)" ]; then \
-		tox -- pep8; \
+	if [ "$(DISABLE_STYLE_VALIDATION)" == "1" ]; then \
+		echo "Warning: style validation disabled, skipping check"; \
 	else \
-		echo "Warning: tox is not installed, skipping check"; \
+		if [ -n "$(TOX)" ]; then \
+			tox -- pep8; \
+		else \
+			echo "Warning: tox is not installed, skipping check"; \
+		fi \
 	fi
 
 .PHONY: python3


-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;

gerrit-hooks has posted comments on this change.

Change subject: tox: added disable tox style validation option to Makefile
......................................................................


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: tox: added disable tox style validation option to Makefile
......................................................................


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: tox: added disable tox style validation option to Makefile
......................................................................


Patch Set 2: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/59417/2//COMMIT_MSG
Commit Message:

Line 10: variable that disables tox run.
Line 11: This option is useful for developers who encounter issues running tox
Line 12: on their environment and want to skip tox style validation.
Line 13: use it by the following:
Line 14: make check DISABLE_STYLE_VALIDATION=1
Why do we need to disable style check?

Adding this flag is clumsy, and make it harder to use the build system.

What we need instead is a new target for running the *only* the tests. If we have such
target, a developer can run exactly what she wants:

    make python3 unitests  # everything but style checks

Or

    make pep8 pyflakes  # only style checks
Line 15: 
Line 16: Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571


-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

Yaniv Bronhaim has posted comments on this change.

Change subject: tox: added disable tox style validation option to Makefile
......................................................................


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/59417/2//COMMIT_MSG
Commit Message:

Line 10: variable that disables tox run.
Line 11: This option is useful for developers who encounter issues running tox
Line 12: on their environment and want to skip tox style validation.
Line 13: use it by the following:
Line 14: make check DISABLE_STYLE_VALIDATION=1
>  Why do we need to disable style check? 
make check can be equal
to make unittests, as it runs recursively check-local in each make file.
In addition to that, we have check-recursive rule -  check-recursive: gitignore
abs_imports python3 pyflakes pep8

we can remove that, but then users will need to call make pep8 pyflakes specifically .. I
still want to keep it in make check. If somehow (she or he) intentionally wants to disable
those checks (internet connection issues or whatever such as edy had), it will be
available by setting the DISABLE_STYLE_VALIDATION flag. imo it fits more to our desire -
each make will try to use and validate our tox jobs, unless specified otherwise.
Line 15: 
Line 16: Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571


-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

Change in vdsm[master]: vdsm.spec: change 'make check' to 'make unittest'

gerrit-hooks has posted comments on this change.

Change subject: vdsm.spec: change 'make check' to 'make unittest'
......................................................................


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: vdsm.spec: change 'make check' to 'make unittest'

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vdsm.spec: change 'make check' to 'make unittest'
......................................................................


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/59417/2//COMMIT_MSG
Commit Message:

Line 10: variable that disables tox run.
Line 11: This option is useful for developers who encounter issues running tox
Line 12: on their environment and want to skip tox style validation.
Line 13: use it by the following:
Line 14: make check DISABLE_STYLE_VALIDATION=1
>  make check can be equal to make unittests, as it runs recursively
> check-loc 
make unitest is *not* equal to make check. It will run the unittests in
tests/, nothing else.
Line 15: 
Line 16: Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571


-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

Change in vdsm[master]: vdsm.spec: change 'make check' to 'make unittest'

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vdsm.spec: change 'make check' to 'make unittest'
......................................................................


Patch Set 3:

(5 comments)

https://gerrit.ovirt.org/#/c/59417/3//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2016-06-18 21:30:59 +0300
Line 4: Commit:     Irit Goihman <igoihman(a)redhat.com&gt;
Line 5: CommitDate: 2016-06-27 16:52:11 +0300
Line 6: 
Line 7: vdsm.spec: change 'make check' to 'make unittest'
We are not replacing make check, but adding a new target that it depend on.
Line 8: 
Line 9: during vdsm rpm check 'make check' is running style validation
Line 10: using tox.
Line 11: not all machines support tox and there are some issues


https://gerrit.ovirt.org/#/c/59417/3/Makefile.am
File Makefile.am:

Line 105
Line 106
Line 107
Line 108
Line 109
This target should depend on unittests.

Since we don't need recursive checks, we can rename it to "check". Hopefully
it will override autoconf recursive check.


Line 88: 		if [ -n "$(TOX)" ]; then \
Line 89: 			tox -- pyflakes; \
Line 90: 		else \
Line 91: 			echo "Warning: tox is not installed, skipping check"; \
Line 92: 		fi \
Not related to adding the new target.
Line 93: 	fi
Line 94: 
Line 95: .PHONY: pep8
Line 96: pep8:


Line 100: 		if [ -n "$(TOX)" ]; then \
Line 101: 			tox -- pep8; \
Line 102: 		else \
Line 103: 			echo "Warning: tox is not installed, skipping check"; \
Line 104: 		fi \
Not related to adding the new target.
Line 105: 	fi
Line 106: 
Line 107: .PHONY: python3
Line 108: python3: all


Line 125: 	$(MAKE) -C tests/ check args='-a type=unit'
Line 126: 
Line 127: .PHONY: unittest
Line 128: unittest:
Line 129: 	$(MAKE) -C tests/ check
Looks fine if we can override check in this makefile so it does not run recursive check on
all directories.
Line 130: 
Line 131: all-local: \
Line 132: 	vdsm.spec
Line 133: 


-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

Change in vdsm[master]: vdsm.spec: change 'make check' to 'make unittest'

Edward Haas has posted comments on this change.

Change subject: vdsm.spec: change 'make check' to 'make unittest'
......................................................................


Patch Set 3:

(2 comments)

https://gerrit.ovirt.org/#/c/59417/2//COMMIT_MSG
Commit Message:

Line 10: using tox.
Line 11: not all machines support tox and there are some issues
Line 12: running tox without network.
Line 13: separating style validation from tests enables creating an rpm
Line 14: without style.
>  make check can be equal to make unittests, as it runs recursively
> check-loc 
How about this:
check: check-style check-tests ...
check-style: pep8 pyflakes
check-tests: check-unit check-integration

I am also in favor of dropping the recursive built-in check and instead explicitly call
the relevant sub-directory.

Having overlapping targets is also not recommend, I would prefer to see each with its own
target and an aggregated target. The disadvantage is that the test-run is separated by
types (unless we can build an args variable per target?).

If we can avoid flags, it will be cleaner IMO.
Line 15: 
Line 16: Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571


https://gerrit.ovirt.org/#/c/59417/3/Makefile.am
File Makefile.am:

PS3, Line 127: unittest
tests includes all kind of tests, not only unit tests.
How about naming the target as 'tests'?


-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

Change in vdsm[master]: vdsm.spec: change 'make check' to 'make unittest'

Irit Goihman has posted comments on this change.

Change subject: vdsm.spec: change 'make check' to 'make unittest'
......................................................................


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/59417/3/Makefile.am
File Makefile.am:

Line 105
Line 106
Line 107
Line 108
Line 109
>  This target should depend on unittests. 
renaming the target
doesn't prevent the recursive call for make check in subdirs. if i add unittest the
tests will run twice


-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

Change in vdsm[master]: Makefile: added 'tests' target

gerrit-hooks has posted comments on this change.

Change subject: Makefile: added 'tests' target
......................................................................


Patch Set 4:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: Makefile: added 'tests' target

Irit Goihman has posted comments on this change.

Change subject: Makefile: added 'tests' target
......................................................................


Patch Set 3:

(5 comments)

https://gerrit.ovirt.org/#/c/59417/3//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2016-06-18 21:30:59 +0300
Line 4: Commit:     Irit Goihman <igoihman(a)redhat.com&gt;
Line 5: CommitDate: 2016-06-27 16:52:11 +0300
Line 6: 
Line 7: vdsm.spec: change 'make check' to 'make unittest'
>  We are not replacing make check, but adding a new target that it
> depend on. 
Done
Line 8: 
Line 9: during vdsm rpm check 'make check' is running style validation
Line 10: using tox.
Line 11: not all machines support tox and there are some issues


https://gerrit.ovirt.org/#/c/59417/3/Makefile.am
File Makefile.am:

Line 88: 		if [ -n "$(TOX)" ]; then \
Line 89: 			tox -- pyflakes; \
Line 90: 		else \
Line 91: 			echo "Warning: tox is not installed, skipping check"; \
Line 92: 		fi \
>  Not related to adding the new target. 
Done
Line 93: 	fi
Line 94: 
Line 95: .PHONY: pep8
Line 96: pep8:


Line 100: 		if [ -n "$(TOX)" ]; then \
Line 101: 			tox -- pep8; \
Line 102: 		else \
Line 103: 			echo "Warning: tox is not installed, skipping check"; \
Line 104: 		fi \
>  Not related to adding the new target. 
Done
Line 105: 	fi
Line 106: 
Line 107: .PHONY: python3
Line 108: python3: all


PS3, Line 127: unittest
>  tests includes all kind of tests, not only unit tests. 
Done


Line 125: 	$(MAKE) -C tests/ check args='-a type=unit'
Line 126: 
Line 127: .PHONY: unittest
Line 128: unittest:
Line 129: 	$(MAKE) -C tests/ check
>  Looks fine if we can override check in this makefile so it does not
> run rec 
Done
Line 130: 
Line 131: all-local: \
Line 132: 	vdsm.spec
Line 133: 


-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

Change in vdsm[master]: Makefile: added 'tests' target

gerrit-hooks has posted comments on this change.

Change subject: Makefile: added 'tests' target
......................................................................


Patch Set 5:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: Makefile: added 'tests' target

Yaniv Bronhaim has posted comments on this change.

Change subject: Makefile: added 'tests' target
......................................................................


Patch Set 5: Code-Review+1

-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: Makefile: added 'tests' target

Edward Haas has posted comments on this change.

Change subject: Makefile: added 'tests' target
......................................................................


Patch Set 5: Code-Review-1

I would suggest splitting this patch into two:
- Override the automake built-in recursive check target, to a non-recursive regular target
and adding a tests target.
- Run only the tests (without style etc..) during rpm build.

-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: Makefile: added 'tests' target

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: Makefile: added 'tests' target
......................................................................


Patch Set 5:

(2 comments)

https://gerrit.ovirt.org/#/c/59417/5//COMMIT_MSG
Commit Message:

Line 6: 
Line 7: Makefile: added 'tests' target
Line 8: 
Line 9: during vdsm rpm check 'make check' is running style validation
Line 10: using tox.
pyflakes is not style validation, please avoid this wrong term, it matches only pep8.
Line 11: not all machines support tox and there are some issues
Line 12: running tox without network.
Line 13: separating style validation from tests enables creating an rpm
Line 14: without style check.


Line 10: using tox.
Line 11: not all machines support tox and there are some issues
Line 12: running tox without network.
Line 13: separating style validation from tests enables creating an rpm
Line 14: without style check.
Same here, don't mention style check.
Line 15: 
Line 16: This patch adds tests as dependency to 'check' target. Also, during
Line 17: rpm build only tests target will run without style validation.
Line 18: 


-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

Change in vdsm[master]: Makefile: added 'tests' target

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: Makefile: added 'tests' target
......................................................................


Patch Set 5:

Irit, you can split the patch to make Edi happy, it is good enough for me after fixing the
commit message.

-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: Makefile: added 'tests' target

gerrit-hooks has posted comments on this change.

Change subject: Makefile: added 'tests' target
......................................................................


Patch Set 6:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: Makefile: added 'tests' target

Irit Goihman has posted comments on this change.

Change subject: Makefile: added 'tests' target
......................................................................


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/59417/5//COMMIT_MSG
Commit Message:

Line 6: 
Line 7: Makefile: added 'tests' target
Line 8: 
Line 9: overrided automake built-in check-recursive target
Line 10: to a non-recursive check and added 'tests' target as
>  pyflakes is not style validation, please avoid this wrong term, it
> matches  
Done
Line 11: a dependency to 'check'.
Line 12: 
Line 13: Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571


-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

Change in vdsm[master]: Makefile: added 'tests' target

Edward Haas has posted comments on this change.

Change subject: Makefile: added 'tests' target
......................................................................


Patch Set 6: Code-Review+1

-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: Makefile: added 'tests' target

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: Makefile: added 'tests' target
......................................................................


Patch Set 6: Code-Review+1

-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: Makefile: added 'tests' target

Irit Goihman has posted comments on this change.

Change subject: Makefile: added 'tests' target
......................................................................


Patch Set 6: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: Makefile: added 'tests' target

[Piotr Kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: Makefile: added 'tests' target
......................................................................


Patch Set 6: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Change in vdsm[master]: Makefile: added 'tests' target

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Makefile: added 'tests' target
......................................................................


Makefile: added 'tests' target

overrided automake built-in check-recursive target
to a non-recursive check and added 'tests' target as
a dependency to 'check'.

Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Signed-off-by: Irit Goihman <igoihman(a)redhat.com&gt;
Reviewed-on: https://gerrit.ovirt.org/59417
Continuous-Integration: Jenkins CI
Reviewed-by: Edward Haas <edwardh(a)redhat.com&gt;
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com&gt;
Reviewed-by: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
---
M Makefile.am
1 file changed, 8 insertions(+), 1 deletion(-)

Approvals:
  Piotr Kliczewski: Looks good to me, approved
  Nir Soffer: Looks good to me, but someone else must approve
  Jenkins CI: Passed CI tests
  Irit Goihman: Verified
  Edward Haas: Looks good to me, but someone else must approve



-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;

Change in vdsm[master]: Makefile: added 'tests' target

gerrit-hooks has posted comments on this change.

Change subject: Makefile: added 'tests' target
......................................................................


Patch Set 7:

* Update tracker: IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

-- 
To view, visit https://gerrit.ovirt.org/59417
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I378dbf38bbce6cadf87fbedbc9bf6a5d1c714571
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No