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

CI(black): Post black code suggestions as PR review comments #3538

Merged
merged 3 commits into from
Mar 29, 2024

Conversation

echoix
Copy link
Member

@echoix echoix commented Mar 27, 2024

Since #3527 is merged, I rebased my implementation of posting code suggestions for Black formatting.
I have implemented a pattern to handle multiple tools in the workflow_run triggered workflow (the one called when a workflow is completed, and that runs with write permissions, and from the context of the default branch->main).

I wrote the workflow with a whitelisting approach in mind, to only allow continuing on the expected file names, in case the workflow is triggered on a file that wasn’t supposed to.
That’s why it’s not a simple loop, that would execute shell scripts on untrusted inputs/interpolation.

@github-actions github-actions bot added the CI Continuous integration label Mar 27, 2024
@echoix echoix added this to the 8.4.0 milestone Mar 27, 2024
@echoix
Copy link
Member Author

echoix commented Mar 28, 2024

Next step: upgrading the black version for the repo, whilst having this tool to help out PRs with the transition.

@echoix
Copy link
Member Author

echoix commented Mar 29, 2024

I'd like to have this reviewed today, so I can chain up the rest during the long weekend. You can see this in action in the grass-addons repo. It worked as intended end of day yesterday.

Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am approving this, but note this is not my area of expertise...

@echoix
Copy link
Member Author

echoix commented Mar 29, 2024

Sorry, I went more with the Python part, but in fact, there's no Python here... it's for Python...

Do you think it is useful though, on a user perspective?

@petrasovaa
Copy link
Contributor

Sorry, I went more with the Python part, but in fact, there's no Python here... it's for Python...

Do you think it is useful though, on a user perspective?

Sorry, I went more with the Python part, but in fact, there's no Python here... it's for Python...

Do you think it is useful though, on a user perspective?

I would hope so, if I understand this correctly, this is most helpful for newcomers and perhaps maintainers.

@echoix echoix merged commit 339e0d1 into OSGeo:main Mar 29, 2024
27 checks passed
@echoix echoix deleted the post-black-suggestions branch May 4, 2024 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants