Unity 8

Merge lp://qastaging/~elopio/unity8/flake8-precommit into lp://qastaging/unity8

flake8-precommit
Merge into trunk

Proposed by Leo Arias on 2014-06-27

Status:	Work in progress
Proposed branch:	lp://qastaging/~elopio/unity8/flake8-precommit
Merge into:	lp://qastaging/unity8
Prerequisite:	lp://qastaging/~elopio/unity8/flake8
Diff against target:	70 lines (+53/-0) 3 files modified .bazaar/Makefile (+1/-0) .bazaar/plugins/run_static_tests.py (+47/-0) run_python_static_tests.sh (+5/-0)
To merge this branch:	bzr merge lp://qastaging/~elopio/unity8/flake8-precommit
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Michael Zanetti (community)			Needs Information on 2014-07-04
Michał Sawicz		2014-06-27	Needs Fixing on 2014-07-04
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2014-06-28
Review via email: mp+224852@code.qastaging.launchpad.net

Commit message

Added a precommit hook for static tests.

Description of the change

* Are there any related MPs required for this MP to build/function as expected? Please list.

The prerequisite branch, https://code.launchpad.net/~elopio/unity8/flake8

* Did you perform an exploratory manual test run of your code change and any related functionality?

Yes. Tried with the hook, without the hook, with the run script, without the run script, with errors and without errors.

* Did you make sure that your branch does not contain spurious tags?

Yes.

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

No packaging changes.

* If you changed the UI, has there been a design review?

No UI changes.

lp://qastaging/~elopio/unity8/flake8-precommit updated on 2014-06-27

981. By Leo Arias on 2014-06-27: Cleaned the call.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-28:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3256/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-07-04:

Hmm we actually wanted a common "pre_push.sh" or so, that would run:
- cmake --build builddir --target test
- flake
- generate and check if qmltypes are updated

Ideally (but tricky without comparing to trunk, so maybe optional...)
- check whether any pngs are being added/modified and remind to use optipng on them

So, please rename the script to be generic, add the cmake check to it, and drop the original hook.

review: Needs Fixing

Revision history for this message

Michael Zanetti (mzanetti) wrote on 2014-07-04:

Do we really need 2 hooks?

I guess it can be a single bzr hook that calls multiple scripts/commands.

afaics the pyflake hook would prevent me from committing if something is not ok. For the other hook we intentionally only print a warning to the user and allow to uncommit, fix, recommit (reusing the commit message) or ignore the hook if pushing to a temporary branch. I'd prefer this one to behave the same, basically just including the python checks (and whatever Michal listed above) in the existing hook.

review: Needs Information

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-07-04:

Yes agreed, the hook should warn, not block.

Unmerged revisions

981. By Leo Arias on 2014-06-27: Cleaned the call.
980. By Leo Arias on 2014-06-27: Improved the hook, added a check for mccabe complexity.
979. By Leo Arias on 2014-06-27: Added the static tests pre-commit hook.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes:

Leo Arias

Mohammad Rakibul Hasan

Unity Team