I run the following bash function at pre-commit https://github.com/wesm/dev-toolchain/blob/1bd3daee1a0b21f8484a7d957de03c640ea8c113/toolchain/arrow-toolchain.sh#L441
Such a thing could be standardized but it may need to be tailored somewhat based on a contributor's focus areas (e.g. mine ignores Rust / Go / Java) On Thu, Feb 20, 2020 at 5:57 AM Joris Van den Bossche <[email protected]> wrote: > > I am personally also not in favor of automatic fixes, but a bot you can > call would be a nice feature. It might be possible to let a workflow be > triggered by an issue comment? ( > https://help.github.com/en/actions/reference/events-that-trigger-workflows#issue-comment-event-issue_comment > ) > > In addition, I think it would be nice to make the pre-commit hooks a > smoother experience. I started using it for pandas, and I really love it > that I don't have to worry anymore about failing CI for some style check > (but, in pandas, the pre-commit hook is simpler of course, since it's only > python checks). > > Joris > > > > On Thu, 20 Feb 2020 at 10:52, Wes McKinney <[email protected]> wrote: > > > I'm also against _automatic_ (user-not-in-the-loop) changes. I think > > the model used conda-forge works pretty well, where a bot lints each > > patch and lets you know if any changes are needed > > > > > > https://github.com/conda-forge/pyarrow-feedstock/pull/99#issuecomment-585264694 > > > > However, note that bot-generated commits could risk running afoul of > > the ASF's code provenance requirements. If a commit to a patch is > > requested / opted-in-to by a contributor then I think there is less of > > an issue, and during the squash everything is rolled up and attributed > > to the contributor(s). > > > > On Thu, Feb 20, 2020 at 3:43 AM Jacek Pliszka <[email protected]> > > wrote: > > > > > > As a beginner contributor I believe I can vote for linting as part of > > the build. > > > > > > For me the best would be BEGINNER/ALL_CHECKS option in the Makefile > > > that does all the linting and all checks done in the build. > > > > > > And in the instruction it would be clearly suggested to use it. > > > > > > BR, > > > > > > Jacek > > > > > > > > > śr., 19 lut 2020 o 20:40 Antoine Pitrou <[email protected]> > > napisał(a): > > > > > > > > > > > > Hi, > > > > > > > > On Wed, 19 Feb 2020 09:59:04 -0800 > > > > > > > > > > It doesn't have to be this way. With GitHub Actions, we can run > > workflows > > > > > that fix style and other violations and push the fix in a commit > > back to > > > > > the branch. > > > > > > > > I'm rather opposed to this. Doing automated pushes behind the user's > > > > back will feel confusing and slightly obnoxious. > > > > > > > > > Style guides and linting are important for large projects like > > Arrow, but > > > > > we don't want to add unnecessary friction to the dev process, > > particularly > > > > > for new contributors--it's challenging enough without it. > > > > > > > > Well, at worse, we can push fixes ourselves before merging a PR if the > > > > only remaining ones are style fixes. > > > > > > > > > What are your thoughts? If anyone objects to using GitHub Actions in > > this > > > > > way, would you be satisfied with blacklisting your fork (i.e. you > > don't > > > > > want it running on your branches but you don't mind if others do)? > > > > > > > > Egoistically, that would satify me, but I'm not sure it would be less > > > > confusing to beginner contributors. > > > > > > > > Regards > > > > > > > > Antoine. > > > > > > > > > >
