search

google / comprehensive-rust

This is the Rust course used by the Android team at Google. It provides you the material to quickly teach Rust.
https://google.github.io/comprehensive-rust/
Apache License 2.0
28.01k stars 1.67k forks source link

Generate PR suggestions when formatting is wrong #2396

Open mgeisler opened 1 month ago

mgeisler commented 1 month ago

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 commented 1 month ago

As noted in https://github.com/parkerbxyz/suggest-changes/issues/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: https://github.com/parkerbxyz/suggest-changes/issues/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 commented 1 month ago

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 (https://github.com/parkerbxyz/suggest-changes/issues/36) instead of a review, but perhaps the email address used can be configured somehow?

parkerbxyz commented 1 month ago

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. 🔜

mgeisler commented 1 month ago

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 https://github.com/parkerbxyz/suggest-changes/issues/41). Then I think we can enable the workflow for our repository.

mgeisler commented 1 month ago

@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.