your patch https://gerrit.ovirt.org/#/c/40346/ broke oVirt vdsm jobs
Patch does not pass pyflakes: ./tests/samplingTests.py:30: 'libvirtconnection' imported but unused ./tests/samplingTests.py:36: 'MonkeyPatch' imported but unused make: *** [pyflakes] Error 1 You could clearly see that the tests did not pass for patchset #6 Please do not merge patches with failing tests! Please fix, Barak Korren oVirt Infra
On Wed, Apr 29, 2015 at 11:16:37AM -0400, Barak Korren wrote:
Patch does not pass pyflakes:
./tests/samplingTests.py:30: 'libvirtconnection' imported but unused ./tests/samplingTests.py:36: 'MonkeyPatch' imported but unused make: *** [pyflakes] Error 1
You could clearly see that the tests did not pass for patchset #6 Please do not merge patches with failing tests!
Barak, thanks for reporting this mistake of ours. https://gerrit.ovirt.org/#/c/40408/ would fix it momentarily. I believe that it stems from two reasons: - Ido did not run `make check` or `make pyflakes` before ticking "verified" on the patch - I failed to wait for the jenkins job to finish. To make sure that this does not repeat I should avoid merging freshly-posted patches. Ido should take better care for pep8 and pyflakes. I have vim plugins that help me avoid such mistakes I hear that http://www.vim.org/scripts/script.php?script_id=4440 is better than what I actually have. Regards, Dan.
----- Original Message -----
From: "Dan Kenigsberg" <danken@redhat.com> To: "Barak Korren" <bkorren@redhat.com> Cc: "Ido Barkan" <ibarkan@redhat.com>, "Francesco Romani" <fromani@redhat.com>, "Infra" <infra@ovirt.org>, "rhev-ci" <rhev-ci@redhat.com> Sent: Thursday, April 30, 2015 12:57:14 AM Subject: Re: your patch https://gerrit.ovirt.org/#/c/40346/ broke oVirt vdsm jobs
On Wed, Apr 29, 2015 at 11:16:37AM -0400, Barak Korren wrote:
Patch does not pass pyflakes:
./tests/samplingTests.py:30: 'libvirtconnection' imported but unused ./tests/samplingTests.py:36: 'MonkeyPatch' imported but unused make: *** [pyflakes] Error 1
You could clearly see that the tests did not pass for patchset #6 Please do not merge patches with failing tests!
Barak, thanks for reporting this mistake of ours. https://gerrit.ovirt.org/#/c/40408/ would fix it momentarily.
I believe that it stems from two reasons: - Ido did not run `make check` or `make pyflakes` before ticking "verified" on the patch - I failed to wait for the jenkins job to finish.
We will configure Jenkins to set -1 to make such mistakes harder to make.
To make sure that this does not repeat I should avoid merging freshly-posted patches. Ido should take better care for pep8 and pyflakes. I have vim plugins that help me avoid such mistakes I hear that http://www.vim.org/scripts/script.php?script_id=4440 is better than what I actually have.
I recommend https://github.com/scrooloose/syntastic its the one ring to rule them all...
Regards, Dan.
--VrqPEDrXMn8OVzN4 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline hi, Dan. makes sense to me to focus on 2 use cases: - pre-commit hook running everything jenkins is running - locally - pros: - nearly identical checks/tests jenkins would running - doesn't care about IDE/editor - cons: - slower - can be annoying to commit (locally) broken code for later squashing - editor/IDE marriage with tests/checks running - pros: - dev has full control over what runs in checks/tests - allows to commit "dirty" commit - shorter ==> quicker than the quickest jenkins option - cons: - depends on IDE/editor support - less checks/tests => higher risk I did both with: intelliJ/PyCharm and vim, almost 100% sure PyDev allows this. either allows ease of running tests - in 1st case upon git commit, in the latter - via a button/shortcut in the devtool. I can help with setting up either to an early adopter. Then give it a week or two to get some feedback later how well it goes. Besides, we're also trying to speedup jenkins response all the time WDYT? On Wed, Apr 29, 2015 at 10:57:14PM +0100, Dan Kenigsberg wrote:
On Wed, Apr 29, 2015 at 11:16:37AM -0400, Barak Korren wrote:
Patch does not pass pyflakes:
./tests/samplingTests.py:30: 'libvirtconnection' imported but unused ./tests/samplingTests.py:36: 'MonkeyPatch' imported but unused make: *** [pyflakes] Error 1
You could clearly see that the tests did not pass for patchset #6 Please do not merge patches with failing tests!
Barak, thanks for reporting this mistake of ours. https://gerrit.ovirt.org/#/c/40408/ would fix it momentarily.
I believe that it stems from two reasons: - Ido did not run `make check` or `make pyflakes` before ticking "verified" on the patch - I failed to wait for the jenkins job to finish.
To make sure that this does not repeat I should avoid merging freshly-posted patches. Ido should take better care for pep8 and pyflakes. I have vim plugins that help me avoid such mistakes I hear that http://www.vim.org/scripts/script.php?script_id=4440 is better than what I actually have.
Regards, Dan.
---end quoted text--- -- Max Kovgan Senior Software Engineer Red Hat - EMEA ENG Virtualization R&D Tel.: +972 9769 2060 Email: mkovgan [at] redhat [dot] com Web: http://www.redhat.com RHT Global #: 82-72060 --VrqPEDrXMn8OVzN4 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVR+1wAAoJEG1jOVLJoOfYVmgQAIr7GLoawCk+M6lujyBldpgt ooU4JgyB5IyCAWtvYSXdslTNkkzQOLuv+JKjBZfWbsc/EviCbF1z9J/9UaB+d0wj guuWRmEgX6P1Y/gne61VeQskXZVGhogR35gtUm3FtVqM9kJeD8TDZ4o+E0Cs6rCh 1jZRJVCFtdKlUJoTucBX6Tfb2xb9wxwNmQNqs6huhh9FP7DmaTD2LOldNCkmtvJP +KKNUTSM6BcoWNz89TpDWfNTAUTgj71Wfdmt+BFn3ucUapRkuYfNFgHSJHjlppBQ YXc8I978mD35VJc8amukMz6nxosRJXlaLCuCuUvmzBmbFP8AErySVfIxLLv8SA09 nkC7L0KxVdWi/AaSpnSkanGIm0G7VficBoZV4WQ+0rL0BjFJVCONHo+T1QNeOV5d M9lSfzJAG/fx6aBT6XbxyAT0ojws4SKziuDemucTh1nzm4dLg07/GKGeEsoHPt4y BJNvkkwEVvWs97ucFAhAmYCtkuclFjRQOx/I3D+yctryEETTMAEXIpYiJVn+PTrF GsgASR5Hf+c4V7/99WD/Dt0qYfxr1VuAqjYKJsh+sEcgZbaiubsTzcKK/S6hmSQQ 1vEuIOAKeER1l/dL00SFnO8rPGU34ZXQK7yXREI7O4/WOalPs1jS6jk0/b8QBZeY LZojwdHrBAN+WE+pSvHK =tS1O -----END PGP SIGNATURE----- --VrqPEDrXMn8OVzN4--
hi, Dan. makes sense to me to focus on 2 use cases: - pre-commit hook running everything jenkins is running - locally Maybe pre-push instead, that will leverage a bit the local work - pros: - nearly identical checks/tests jenkins would running - doesn't care about IDE/editor - cons: - slower - can be annoying to commit (locally) broken code for later squashing =20 - editor/IDE marriage with tests/checks running - pros: - dev has full control over what runs in checks/tests - allows to commit "dirty" commit - shorter =3D=3D> quicker than the quickest jenkins option - cons: - depends on IDE/editor support - less checks/tests =3D> higher risk =20 I did both with: intelliJ/PyCharm and vim, almost 100% sure PyDev allows =
--YhFoJY/gx7awiIuK Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 05/05, Max Kovgan wrote: this.
=20 either allows ease of running tests - in 1st case upon git commit, in the latter - via a button/shortcut in the devtool. I can help with setting up either to an early adopter. Then give it a week or two to get some feedback later how well it goes. =20 Besides, we're also trying to speedup jenkins response all the time =20 WDYT? =20 =20 =20 =20 On Wed, Apr 29, 2015 at 10:57:14PM +0100, Dan Kenigsberg wrote:
On Wed, Apr 29, 2015 at 11:16:37AM -0400, Barak Korren wrote:
Patch does not pass pyflakes:
./tests/samplingTests.py:30: 'libvirtconnection' imported but unused ./tests/samplingTests.py:36: 'MonkeyPatch' imported but unused make: *** [pyflakes] Error 1
You could clearly see that the tests did not pass for patchset #6 Please do not merge patches with failing tests!
Barak, thanks for reporting this mistake of ours. https://gerrit.ovirt.org/#/c/40408/ would fix it momentarily.
I believe that it stems from two reasons: - Ido did not run `make check` or `make pyflakes` before ticking "verified" on the patch - I failed to wait for the jenkins job to finish.
To make sure that this does not repeat I should avoid merging freshly-posted patches. Ido should take better care for pep8 and pyflakes. I have vim plugins that help me avoid such mistakes I hear that http://www.vim.org/scripts/script.php?script_id=3D4440 is better than what I actually have.
Regards, Dan.
---end quoted text--- =20 -- Max Kovgan =20 Senior Software Engineer Red Hat - EMEA ENG Virtualization R&D Tel.: +972 9769 2060 Email: mkovgan [at] redhat [dot] com Web: http://www.redhat.com RHT Global #: 82-72060
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
--=20 David Caro Red Hat S.L. Continuous Integration Engineer - EMEA ENG Virtualization R&D Tel.: +420 532 294 605 Email: dcaro@redhat.com Web: www.redhat.com RHT Global #: 82-62605 --YhFoJY/gx7awiIuK Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVSHsdAAoJEEBxx+HSYmnDgGMH/14wSRAlcNGQlThBM98PkQG1 cwjVBVTaHUdr2gngaMoxDyR5V7uou99WYMYNnJDM5zcTlqulaN2NMAkouKtHeCJs bAJkWjKCFyAnvvFUpX/y6Ktjt+0JlO5NvhG4Hreq4iC+YafJ8luPAgV/5RJ/8b2P p1dAdnt/G0qykarecPYNL65iteLQfT/WZSI59m8xfyrAqGch9Hhm0OPSaNILvg3l Yw3iPbXiAMbpOkcuVStY3+oJiD4OEKvphiQ8RUWhqhw2MWavlE78+7hlVQXxW57A aO6I/JHHHC4GKdmXIT0Oi0Q1PJ8p5zrJ4Xa57JSHoumzlkR/RGik/zTNbhb1I6U= =+Ewf -----END PGP SIGNATURE----- --YhFoJY/gx7awiIuK--
On Tue, May 05, 2015 at 10:11:09AM +0200, David Caro wrote:
On 05/05, Max Kovgan wrote:
hi, Dan. makes sense to me to focus on 2 use cases: - pre-commit hook running everything jenkins is running - locally Maybe pre-push instead, that will leverage a bit the local work - pros: - nearly identical checks/tests jenkins would running - doesn't care about IDE/editor - cons: - slower - can be annoying to commit (locally) broken code for later squashing
If something is too anoying to be run (such as blocking every patch for 3 minute unit tests, when the poor developer only wants to post his patch and go home) - developer would find a way to skip it.
- editor/IDE marriage with tests/checks running - pros: - dev has full control over what runs in checks/tests - allows to commit "dirty" commit - shorter ==> quicker than the quickest jenkins option - cons: - depends on IDE/editor support - less checks/tests => higher risk
+1. It boils down to developer and maintainer prudence. I have such a plugin in my ViM for static testing; Ido (and everyone else) should have one, too. I'm less sure about auto-running `make check` at rundom points in time.
I did both with: intelliJ/PyCharm and vim, almost 100% sure PyDev allows this.
either allows ease of running tests - in 1st case upon git commit, in the latter - via a button/shortcut in the devtool. I can help with setting up either to an early adopter. Then give it a week or two to get some feedback later how well it goes.
Besides, we're also trying to speedup jenkins response all the time
I would not mind to BLOCK merging before jenkins hook has responded - assuming that I (as a branch maintainer) can remove the jenkins reviewer from gerrit. There could be emenrgencies that cannot wait for the response. And of course, as a maintainer, I must be able to override the decision of the robot (by removing it from the reviewer list).
On 05/05, Max Kovgan wrote:
hi, Dan. makes sense to me to focus on 2 use cases: - pre-commit hook running everything jenkins is running - locally Maybe pre-push instead, that will leverage a bit the local work - pros: - nearly identical checks/tests jenkins would running - doesn't care about IDE/editor - cons: - slower - can be annoying to commit (locally) broken code for later squa= shing =20 If something is too anoying to be run (such as blocking every patch for 3 minute unit tests, when the poor developer only wants to post his
On Tue, May 05, 2015 at 10:11:09AM +0200, David Caro wrote: patch and go home) - developer would find a way to skip it. =20
=20 - editor/IDE marriage with tests/checks running - pros: - dev has full control over what runs in checks/tests - allows to commit "dirty" commit - shorter =3D=3D> quicker than the quickest jenkins option - cons: - depends on IDE/editor support - less checks/tests =3D> higher risk =20 +1. It boils down to developer and maintainer prudence. I have such a plugin in my ViM for static testing; Ido (and everyone else) should have one, too. I'm less sure about auto-running `make check` at rundom points in time. =20
I did both with: intelliJ/PyCharm and vim, almost 100% sure PyDev all= ows this. =20 either allows ease of running tests - in 1st case upon git commit, in=
--Bu8it7iiRSEf40bY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 05/05, Dan Kenigsberg wrote: the
latter - via a button/shortcut in the devtool. I can help with setting up either to an early adopter. Then give it a week or two to get some feedback later how well it goe= s. =20 Besides, we're also trying to speedup jenkins response all the time =20 I would not mind to BLOCK merging before jenkins hook has responded - assuming that I (as a branch maintainer) can remove the jenkins reviewer from gerrit. There could be emenrgencies that cannot wait for the response. And of course, as a maintainer, I must be able to override the decision of the robot (by removing it from the reviewer list).
I'm actually working on adding a new flag 'Continuous Integration' that can only be set by maintainers and the ci bot, and that requires +1 to merge (w= here -1 does not block). Does that make sense to you? (that way you can't rebase and merge before ci runs and -1, it's easier to handle permissions, it's easier to spot on the = ui, is clearer it's purpose and does not overload another flag).
=20
--=20 David Caro Red Hat S.L. Continuous Integration Engineer - EMEA ENG Virtualization R&D Tel.: +420 532 294 605 Email: dcaro@redhat.com Web: www.redhat.com RHT Global #: 82-62605 --Bu8it7iiRSEf40bY Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVSN8oAAoJEEBxx+HSYmnDo7YH/1uZzyxxNrIndJniR9kFOKci urqg5LFXLt9BKT9gN1xDnoubV9ssRd3Y17RFxwUeoBHPJ9PCfUg6Da33rIDQjUbh xVxlD7LDuig318q8vkQ5z6FwV38f/4f60jYZ2re5fkTuECGQqY1OaJ9cH2H+7Qta X44vfDGR1i/XeK/EoIgQPJti2NN7eGM6FC6Y/8+pchRGM+kY6m3gwnA+DmL/H/ly qcEzRx5SdQawfWnGHcgCOe2AAtHSPFhMJ6rDqrHW1WBBid2/IbG9mM3NkI59Q/8W 5zDkUSlISUske+C/wHyWGGVAjY4WEw01SVv8EPx0fwDAfxLRRtdPhkiqNxlvGeI= =d6wh -----END PGP SIGNATURE----- --Bu8it7iiRSEf40bY--
On Tue, May 05, 2015 at 05:18:00PM +0200, David Caro wrote:
On 05/05, Dan Kenigsberg wrote:
On Tue, May 05, 2015 at 10:11:09AM +0200, David Caro wrote:
On 05/05, Max Kovgan wrote:
hi, Dan. makes sense to me to focus on 2 use cases: - pre-commit hook running everything jenkins is running - locally Maybe pre-push instead, that will leverage a bit the local work - pros: - nearly identical checks/tests jenkins would running - doesn't care about IDE/editor - cons: - slower - can be annoying to commit (locally) broken code for later squashing
If something is too anoying to be run (such as blocking every patch for 3 minute unit tests, when the poor developer only wants to post his patch and go home) - developer would find a way to skip it.
- editor/IDE marriage with tests/checks running - pros: - dev has full control over what runs in checks/tests - allows to commit "dirty" commit - shorter ==> quicker than the quickest jenkins option - cons: - depends on IDE/editor support - less checks/tests => higher risk
+1. It boils down to developer and maintainer prudence. I have such a plugin in my ViM for static testing; Ido (and everyone else) should have one, too. I'm less sure about auto-running `make check` at rundom points in time.
I did both with: intelliJ/PyCharm and vim, almost 100% sure PyDev allows this.
either allows ease of running tests - in 1st case upon git commit, in the latter - via a button/shortcut in the devtool. I can help with setting up either to an early adopter. Then give it a week or two to get some feedback later how well it goes.
Besides, we're also trying to speedup jenkins response all the time
I would not mind to BLOCK merging before jenkins hook has responded - assuming that I (as a branch maintainer) can remove the jenkins reviewer from gerrit. There could be emenrgencies that cannot wait for the response. And of course, as a maintainer, I must be able to override the decision of the robot (by removing it from the reviewer list).
I'm actually working on adding a new flag 'Continuous Integration' that can only be set by maintainers and the ci bot, and that requires +1 to merge (where -1 does not block).
Does that make sense to you? (that way you can't rebase and merge before ci runs and -1, it's easier to handle permissions, it's easier to spot on the ui, is clearer it's purpose and does not overload another flag).
Yes, it does!
participants (4)
-
Barak Korren -
Dan Kenigsberg -
David Caro -
Max Kovgan