Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate PR suggestions when formatting is wrong #2396

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mgeisler
Copy link
Collaborator

@mgeisler mgeisler commented Oct 5, 2024

This should make it much easier to do drive-by changes in the GitHub editor: the comment should contain a diff that can be committed directly from the online editor.

This should make it much easier to do drive-by changes in the GitHub
editor: the comment should contain a diff that can be committed
directly from the online editor.
@mgeisler mgeisler force-pushed the pr-formatting-suggestions branch from 505dd6c to 223d565 Compare October 5, 2024 12:04
@mgeisler mgeisler marked this pull request as draft October 5, 2024 12:04
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
README.md Outdated Show resolved Hide resolved
@mgeisler mgeisler force-pushed the pr-formatting-suggestions branch from d0b15c0 to 3df2707 Compare October 5, 2024 12:08
@mgeisler mgeisler dismissed github-actions[bot]’s stale review October 5, 2024 12:10

Formatting changes have been applied

@mgeisler
Copy link
Collaborator Author

mgeisler commented Oct 5, 2024

As noted in parkerbxyz/suggest-changes#37, the action creates a new review every time the PR is modified. So if the formatting errors are not resolved right away, a new comment is added repeatedly.

I think we can live with that: people will hopefully quickly apply the formatting suggestion and so the comments should not be too burdensome.

I'm not 100% sure if this works then a PR comes from a fork: parkerbxyz/suggest-changes#33

A final problem is the stickiness of the review left by the action. After all formatting problems have been resolved, the negative review will need to be dismissed by hand before the PR can be merged. The burden of this is on the PR reviewers, so this is probably acceptable?

@mgeisler mgeisler marked this pull request as ready for review October 5, 2024 12:18
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@mgeisler mgeisler force-pushed the pr-formatting-suggestions branch from 3df2707 to 23fc1fb Compare October 6, 2024 14:10
@mgeisler
Copy link
Collaborator Author

mgeisler commented Oct 6, 2024

Oh no, there is another problem: the suggestions are attributed to the github-actions bot! This makes the CLA fail:

image

I think this would be the same, even if comments are used (parkerbxyz/suggest-changes#36) instead of a review, but perhaps the email address used can be configured somehow?

.github/workflows/build.yml Outdated Show resolved Hide resolved
@parkerbxyz
Copy link

As noted in parkerbxyz/suggest-changes#37, the action creates a new review every time the PR is modified. So if the formatting errors are not resolved right away, a new comment is added repeatedly.

This has been fixed in the latest release.

After all formatting problems have been resolved, the negative review will need to be dismissed by hand before the PR can be merged.

I'd like to make the review action configurable so you can have it just leave comments without blocking merging. 🔜

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@mgeisler
Copy link
Collaborator Author

This has been fixed in the latest release.

Thanks @parkerbxyz, that sounds great!

I will need to change the email address used for the suggestions (for the CLA check, as discussed in parkerbxyz/suggest-changes#41). Then I think we can enable the workflow for our repository.

@mgeisler
Copy link
Collaborator Author

@djmitche, we have documentation for setting up and registering a bot account here: go/github-docs/robots. I will probably go through this process to get a custom GITHUB_TOKEN we can use in the workflows.

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.

3 participants