Auto formatting via pre-commit.ci

[PointKernel]

This PR uses pre-commit.ci to enable automatic style fixing for commits in a PR. Users should allow edits by maintainers to get auto-formatting to work.

[PointKernel]

pre-commit.ci run

[PointKernel]

pre-commit.ci run

[bdice]

Did this trigger the CI bot correctly? I don’t see any commits from it.

[bdice]

FYI, this pinning will probably be updated by the “autoupdate” functionality of pre-commit.ci.

[PointKernel]

Right, this is why I set the update frequency to quarterly. And according to pre-commit-ci/issues#83, there is no plan to provide a schedule-off option.

[bdice]

I support quarterly just to keep the number of PRs down -- but it does seem like we could update the pinnings before we merge this PR, since it's going to keep clang-format updated anyway. (Unless we're trying to stay pinned to v11.1.0 for some reason? I don't think the choice of version matters if it's automatically enforced.)

[PointKernel]

Done

[vyasr]

Do we want to keep this version in line with RAPIDS? AFAIK most cuCollections developers (at least at present) are RAPIDS devs, and having this misaligned could lead to issues if users choose to invoke clang-format via the CLI and they have a different version installed than the one specified by pre-commit. I would be fine if the answer is no, but we should be aware of the issues this discrepancy could cause.

[bdice]

I don't think the choice of version matters if it's automatically enforced.

That’s my point: any issues caused by misaligned versions will be auto-fixed by CI, so it doesn’t matter what devs have locally. Moreover, you can’t really pin the version because pre-commit CI will open a PR to update it to the latest version every quarter. Personally, I am perfectly okay with keeping the pinnings updated on a quarterly basis and ignoring whatever devs have locally in favor of the pre-commit autofixes.

If we implemented our own “auto-fix” solution in gpuCI similar to pre-commit.ci, then we’d have greater control over version pinnings. However, I don’t think that is needed.

[PointKernel]

I agree with @bdice, any version mismatch issues will be auto fixed by CI anyway. By all means, it's clearly stated in README that directly using clang-format on the command line is not recommended.

[vyasr]

To be clear, I understood what @bdice was saying about how things would work. I'm asking for a value judgment on whether we think this is a problem. We explicitly declined to move in this direction for RAPIDS because it would prevent using the format-on-write hooks in VSCode, which can't use the pre-commit hook. I don't personally use VSCode, so I don't care about that feature, but before we commit to this approach I just wanted to make sure that everyone involved recognized the limitations since lack of VSCode integration was deemed sufficient reason not to rely on pre-commit versioning in RAPIDS. I'm happy if we just stick with pre-commit though :)

[PointKernel]

I see your point. This could be a problem not only for VSCode but for all IDEs/editors having auto-formatting support. Technically, users can still stick with the IDE/eidtor clang-format hook. It's just any mismatches will be then auto-fixed by CI (or pre-commit hook).

[bdice]

It's just any mismatches will be then auto-fixed by CI (or pre-commit hook).

I'm strongly in favor of trying this approach here. If it works well, I want to eventually revisit that conversation for RAPIDS or cuDF in particular. I don't expect it to be painful, and I suspect it will lead to greater productivity than our current approach of waiting for CI to fail and then identifying/fixing/committing/pushing "style fixes." The current manual approach costs us so much in CI retry time and hinders our iteration speed. Lately I've been doing a lot of work via github.dev (to make small changes, since it's vastly quicker than dealing with local branches / etc. and I can work in lots of browser tabs at once) and automatic CI fixes for style would be so awesome to add to that workflow.