Are you still thinking precommit hook there? I usually prefer server side hooks as then I don't have to deploy all the developer tools all of the random repos I contribute to require all the time. If it can be made server side somehow, thats vastly easier to deal with by folks like me or other drive by contributors. I'm thinking more along the line of, we have a bunch of gate blocking tests that ensure some amount of code quality going in. But what if we could take it to the next level and have those tests actually feed back quality enhancements too, not just saying no and without the developer to install extra tools? Thanks, Kevin ________________________________________ From: Sean Mooney [smooney@redhat.com] Sent: Tuesday, April 23, 2019 1:13 PM To: Fox, Kevin M; Jeremy Stanley; openstack-discuss@lists.openstack.org Subject: Re: The python black project. On Tue, 2019-04-23 at 19:56 +0000, Fox, Kevin M wrote:
Thats a slightly different thing, as it modifies the user contribution on the way in. That arguably breaks signoffs and the like, as well as git history for someone pushing vs pulling I think?
Feeding back the patchs though so the user doesn't have to write them, but easly can accept them I think is a better middle ground? It automates the time consuming parts, but goes through the workflow in such a way as not to break other things? well what i was thinking was we would use the return code of autopep8 as the job sucess
--exit-code change to behavior of exit code. default behavior of return value, 0 is no differences, 1 is error exit. return 2 when add this option. 2 is exists differences. so in the gate we coudl invoke "tox -e autopep8 -- --diff > result.diff" and just use the execode to know if it woudl change somehting. if it exeits 0 then the job passes if it exits 1 then it fails and the the user can grab the diff an apply it or they coudl just run "tox -e autopep8 -- --in-place" then in either case the locally git add and git commit --amend and resubmit to make sure authership is preserved. the autopep8 tox enve could also automate runing it on only the files changed in the current commit to minimise chrun like the fast8 env does. that would allow use to fix things incrementlaly as we go. i might just code this up but i dont really want to waste time on it if it will never merge.
Thanks, Kevin ________________________________________ From: Sean Mooney [smooney@redhat.com] Sent: Tuesday, April 23, 2019 12:22 PM To: Fox, Kevin M; Jeremy Stanley; openstack-discuss@lists.openstack.org Subject: Re: The python black project.
On Tue, 2019-04-23 at 19:06 +0000, Fox, Kevin M wrote:
I've wondered for a while if you could automate the automation. So, like, run the autoformatter and feed back the generated patch back to the developer in a way they can easily accept it. Like, making a pr against their pr with the formatting fixes, so that they can easily merge it in.
well you can with a pre-commit hook + a job chagne to enforce it. so pre-commit hook runs an autopep8 tox env. if the tox run modifies any files fail the commit. in the gate we also assert the same thing in the pep8 job. and retrun the diff in the logs somewhere
anyway i know not everyone would be on board with that but its what i would proably do if i was starting proejct from scratch.
Thanks, Kevin ________________________________________ From: Jeremy Stanley [fungi@yuggoth.org] Sent: Tuesday, April 23, 2019 11:39 AM To: openstack-discuss@lists.openstack.org Subject: Re: The python black project.
On 2019-04-23 15:39:48 +0100 (+0100), Sean Mooney wrote: [...]
ya i think i could get more behind autopep8 or yapf. running autopep8 on nova https://review.opendev.org/#/c/655171/ has very little change
litrally adding 1 empty new line to to 102 files that with almost 0 other code curn. using autopep8 however would fix any actul pep8 issue in new patches automatically without the downsides of black.
[...]
I'm more concerned with the implementation logistics, honestly. How do you propose going about this? If you want to use a Git commit or pre-commit hook then you can do it already--go for it. If you want everyone proposing changes to the same repository to use the same commit hook, that becomes a distribution/enforcement challenge to solve. If you want it somehow enforced on the receiving end of the push, say with a receive hook, that's brittle and will have side effects such as invalidating signed commits (which we're able to support at the moment).
Basically the only friendly and reliable way to enforce this centrally is to have the desired style conventions validated upon receipt by the code review system and the results reported back to the committer. This is exactly what we already do today. There are of course also ways to reject the change with a push error so that the committer has to amend and push again, but that's not easy to manage across different projects and also doesn't solve your desire to "not need to think about code style" (paraphrased).
There is also the possibility that you simply run a periodic autopep8 job against the codebase and have that job propose a commit to the repository to apply code style changes if there are any, but that's a bit of a messy hack as well. -- Jeremy Stanley