Black only runs in python 3.6 and up. We can use it to format and check 2.7 and 3.5, but it will not run in our existing openjdk based docker image without significant modifications.
That and the fact that we are primarily interested in checking, not changing, are why I think pycodestyle is the “right choice”, or at least one I can get done without a major effort. If you want me to pursue integrating black let me know so we can discuss the changes required. On Wed, May 29, 2019 at 04:29 Ismaël Mejía <[email protected]> wrote: > +1 to what Fokko said, Black looks like the right tool. I did a quick > check and the python 3 codebase looks healthy for Black however there > will be some work to do in the python 2 one. > > On Wed, May 29, 2019 at 9:04 AM Driesprong, Fokko <[email protected]> > wrote: > > > > Why change to pycodestyle? We can still use black for this, right? The > > command: > > black --check --diff . > > will throw a non-zero exit code when there is something to format. > > > > Cheers, Fokko > > > > > > > > Op wo 29 mei 2019 om 04:04 schreef Michael A. Smith < > [email protected]>: > > > > > If all we want is the diff/exit code, then we should use pycodestyle > > > instead of black. I'll change course to work on that. > > > > > > On Tue, May 28, 2019 at 5:24 AM Driesprong, Fokko <[email protected] > > > > > wrote: > > > > > > > Fully agree with you Ismaël. > > > > > > > > > > > > The issue is that it CAN write directly. For example here: > > > > https://github.com/Fokko/avro/pull/35 If you ask dependabot to > merge it, > > > > it > > > > will be merged into master. And therefore it will ask for write > access to > > > > the repository. Recently dependabot has been acquired > > > > <https://dependabot.com/blog/hello-github/> by Github, so I guess > we can > > > > ask Github to sign a CLA :-) It is worth a shot of discussing this > > > upstream > > > > in the ASF. Dependabot letting us know when there is a library with > known > > > > CVE's will make the software much saver. > > > > > > > > Cheers, Fokko > > > > > > > > Op di 28 mei 2019 om 10:46 schreef Ismaël Mejía <[email protected]>: > > > > > > > > > +1 for Black, great idea and in line we the changes we did with > > > > > spotless for the Java code base. > > > > > > > > > > About dependabot I think this is an interesting case to discuss in > > > > > upstream apache lists. So far ASF allows bots that do not touch the > > > > > code, for example for metadata, as we do with the autolabeler bot. > If > > > > > I undertand correctly dependabot does not change the code, it opens > > > > > Pull Requests and it is up to a committer to decide or not if the > code > > > > > is good. Or does it write directly? > > > > > > > > > > I suppose that even if technically is possible, there could be an > > > > > authorship issue to be discussed. Can bots sign an ICLA :D living > in > > > > > the future mates! > > > > > > > > > > On Tue, May 28, 2019 at 10:01 AM Driesprong, Fokko > > > <[email protected] > > > > > > > > > > wrote: > > > > > > > > > > > > Thanks, Michael for working on this. I think having an auto > formatter > > > > for > > > > > > Python is valuable since it will decrease the conflicts in the > > > future. > > > > > For > > > > > > now, we need to do a big PR to get all the files in the correct > > > format. > > > > > > > > > > > > We need to add to the CI: black --check --diff . This will throw > a > > > > > non-zero > > > > > > exit code if there is something to format. So that the author of > the > > > PR > > > > > > needs to apply black to let the CI pass. We should make this > part of > > > > the > > > > > > build.sh of the python3 project. > > > > > > > > > > > > I don't think that such a CI service exists, and I also think it > is a > > > > bad > > > > > > idea. The author should format his code on forehand. > > > > > > > > > > > > I've also looked in setting up Dependabot for Avro, but this is > > > against > > > > > the > > > > > > Apache rules because the Dependabot integration requires write > > > > > permissions > > > > > > on the repository which isn't allowed. Hope this helps. > > > > > > > > > > > > Cheers, Fokko > > > > > > > > > > > > Op di 28 mei 2019 om 03:39 schreef Michael A. Smith < > > > > > [email protected]>: > > > > > > > > > > > > > I am working on making all the py and py3 code consistent with > > > > > > > https://github.com/python/black, but once done it’d be great > if we > > > > > could > > > > > > > keep it consistent. I will look into adding hooks and stuff for > > > > > > > yetus/TravisCI, but is there a way to have an automation that > can > > > > > > > periodically do all the formatting for us, and open pull > requests > > > > with > > > > > any > > > > > > > changes required? > > > > > > > > > > > > > > I mean like dependabot, but instead of opening a pr to update > > > > > dependencies, > > > > > > > it opens a pr that does isort, black, docformatter, or > whatever we > > > > > want. > > > > > > > > > > > > > > This way, we get consistent style without it being an > “enforcement > > > > > > > priority” in prs by humans. If someone has a valuable > contribution, > > > > we > > > > > > > don’t have to do a back-and-forth with them about style and > > > > formatting. > > > > > > > > > > > > > > If there’s interest I could look into implementing something > with > > > > > existing > > > > > > > CI tools, or using github actions. > > > > > > > > > > > > > > What do y’all think? > > > > > > > > > > > > > > > > > > > >
