Skip to content

Conversation

@chriskrycho
Copy link
Contributor

Introduce a GitHub Action which allows us to add a comment !format to a given PR to have its contents automatically formatted with dprint. This lets contributors not worry about our exact formatting guidelines and focus instead on contributing. People who want to go the extra mile can, and we document local autoformatting setup in CONTRIBUTING.md, but this way online-only contributions can “just work”.

WIP because this is one alternative; we may also:

  • Use one of the existing Rust infra bots.
  • Do this but include merge capability so we don’t have to do an extra dance when merging, just !format-and-merge and have it do both.
  • Nothing!

Introduce a GitHub Action which allows us to add a comment `!format` to
a given PR to have its contents automatically formatted with dprint.
This lets contributors not worry about our exact formatting guidelines
and focus instead on contributing. People who *want* to go the extra
mile can, and we document local autoformatting setup in CONTRIBUTING.md,
but this way online-only contributions can “just work”.

WIP because this is one alternative; we may also:

- Use one of the existing Rust infra bots.
- Do this but include merge capability so we don’t have to do an extra
  dance when merging, just `!format-and-merge` and have it do both.
- Nothing!
@chriskrycho
Copy link
Contributor Author

Okay, a bit of further investigation suggests that doing it via an Action on pull requests is probably not the right path forward:

  1. It isn’t guaranteed even to be possible: any individual contributor gets to choose whether maintainers are allowed to push to their fork/branch. Note that this also precludes the other bot approaches we discussed here, as far as I can tell.

  2. To even be able to push to a fork or branch when users do allow it, we would need a dedicated GitHub access token, because the default GITHUB_TOKEN only has the read permission on a fork, and it would need a write permission (specifically on content, but that detail doesn’t matter here).

Net, I think we have to do it either on push (which folks on the Rust infra team are, quite reasonably, a bit uncomfortable with) or add a check to force folks to clone and run it (which I don’t want to do for the reasons noted above) if we want it to be kept up to date continually.

We can also generate file annotations without blocking merge, parsing the output of dprint check along with a tiny GitHub Action script to create the annotations.

Net, though, it’s not that important—there is no semantic meaning to it, after all—so if folks are uncomfortable just doing it on push, I’ll probably just stick to documenting it (#4138) and doing it manually at this point.

@chriskrycho chriskrycho closed this Dec 5, 2024
@chriskrycho chriskrycho deleted the format-auto-action-wip branch December 5, 2024 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant