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!

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.

-----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-----

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

> > - 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

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).