-
Notifications
You must be signed in to change notification settings - Fork 6
Improve action that manages issue headers #28
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
Conversation
0c7e9fa to
d27caa4
Compare
- Move shared part to a separate file - Trigger on open new issue event - Remove header from templates
d27caa4 to
9472b9a
Compare
rtibbles
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes make sense - a couple of comments related to events.
|
|
||
| on: | ||
| issues: | ||
| types: [opened, labeled, unlabeled] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to cover some edge cases we can add reopened and transferred to this list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, thank you. I only added reopened here. I played around with transferred and the way it works is that a receiving repository will run opened event on issues that are transferred into it, so it's covered. However I updated the script to handle better both reopened and transferred issues. Tested successfully.
| manage-issue-header: | ||
| runs-on: ubuntu-latest | ||
| if: | | ||
| github.event.action == 'opened' || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would need to add the other event names here too, presumably.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thanks!
2ce9708 to
3251d3d
Compare
|
@rtibbles Companion pull requests:
We should merge this one and the five above together. |
|
@rtibbles I also pushed upgrade of dependencies and caching of node dependencies here. Tested the latest version on |
| - name: Install dependencies | ||
| run: npm install | ||
| run: yarn install --frozen-lockfile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is yarn's version of npm ci - I've recently learned it's recommended approach in workflows. This is also a a reason why I am committing yarn.lock. When I have a while, I will upgrade and improve other community related workflows similarly.
Summary
Improve action that manages issue headers:
I've developed and previewed in my replica of LE's GitHub, and also tested in
test-actions40caf0f.References
Follow-ups for @rtibbles recent feedback.
Reviewer guidance
Please don't merge. After approval, I will open companion PRs in the five repositories, and then we can merge everything together.