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

dev: add action to add checklist for new linter #4855

Merged
merged 3 commits into from
Jul 3, 2024

Conversation

bombsimon
Copy link
Member

This will add the checklist for new linters to the repository and revision control. When the label linter: new is added the checklist will automatically be added to the PR instead of having to copy-paste it from an arbitrary source.

Since only collaborators and members can add labels it's not likely that this will be abused, however to mitigate the risk even further the action will reset the checklist comment if the tag is added and removed multiple times.

@bombsimon bombsimon force-pushed the new-lint-checklist branch from 7d884ce to 7e3919a Compare July 2, 2024 20:08
@ldez ldez self-requested a review July 2, 2024 21:29
@ldez ldez added the area: ci PR that update CI label Jul 2, 2024
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

I'm not a fan of labeled events with an if because the workflow will be triggered for all the labeled events and will create "noise" inside the GitHub action tab.
Sadly, it's not possible to define the type inside the on section.

This is the main reason why I never created this workflow.

An alternative is to use workflow_dispatch.

.github/workflows/new_linter_checklist.yaml Outdated Show resolved Hide resolved
@ldez ldez added the feedback required Requires additional feedback label Jul 2, 2024
@ldez ldez changed the title Add action to add checklist for new linter dev: add action to add checklist for new linter Jul 2, 2024
@bombsimon
Copy link
Member Author

I'm not a fan of labeled events with an if because the workflow will be triggered for all the labeled events and will create "noise" inside the GitHub action tab. Sadly, it's not possible to define the type inside the on section.

Makes sense! I can address your concern about the markdown file but if you don't think this is useful I think we can close it instead. Didn't know you considered it and opted not to do it.

An alternative is to use workflow_dispatch.

Yeah maybe, it would add the list to the repo but would require a manual invocation so not sure if it's useful. I also don't know how to target a specific PR from a workflow_dispatch, didn't know you could add context for a singular PR.

@ldez
Copy link
Member

ldez commented Jul 3, 2024

The best solution (if we except a better labeling event management) is to have "saved replies" by repository or organization, but GitHub doesn't seem to plan to add this feature:

Yeah maybe, it would add the list to the repo but would require a manual invocation so not sure if it's useful

At least the checklist template will be available for all the maintainers.
But yes workflow_dispatch is not the best solution.

@ldez
Copy link
Member

ldez commented Jul 3, 2024

I modified your PR to use workflow_dispatch: the PR number is defined during the launch of the workflow.

example

FYI, I tested the workflow with a testing repo (ldez/redesignedwaffle#2)

@ldez ldez force-pushed the new-lint-checklist branch from 7b41b95 to 7e7e404 Compare July 3, 2024 13:38
@bombsimon
Copy link
Member Author

I modified your PR to use workflow_dispatch: the PR number is defined during the launch of the workflow.

Nice! Never used inputs so TIL.

Since 100% of the uses of this checklist have been from you so far feel free to decide if you want this as an action or keep doing it the old style! If the latter just close this PR!

Another goal with the action was to add the comment as soon as possible without human interaction for the author of the PR to quickly see what's expected when adding a new linter. That won't be the case now so maybe just more work for you to trigger the workflow. 😕

@ldez
Copy link
Member

ldez commented Jul 3, 2024

The approach allows sharing the comment content with all maintainers, it's positive.
The workflow dispatch allows all maintainers to set the comment "easily", it's also positive.

Currently, I will still use my "Saved Reply" in most cases because it's easier for me, but I will migrate to the workflow dispatch.

bombsimon and others added 3 commits July 3, 2024 20:59
This will add the checklist for new linters to the repository and
revision control. When the label `linter: new` is added the checklist
will automatically be added to the PR instead of having to copy-paste it
from an arbitrary source.

Since only collaborators and members can add labels it's not likely that
this will be abused, however to mitigate the risk even further the
action will reset the checklist comment if the tag is added and removed
multiple times.
@bombsimon bombsimon force-pushed the new-lint-checklist branch from 7e7e404 to 7d982df Compare July 3, 2024 18:59
@ldez ldez added enhancement New feature or improvement and removed feedback required Requires additional feedback labels Jul 3, 2024
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

@ldez ldez merged commit 8f348db into golangci:master Jul 3, 2024
16 checks passed
@ldez ldez added this to the v1.60 milestone Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ci PR that update CI enhancement New feature or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants