-
-
Notifications
You must be signed in to change notification settings - Fork 23
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
Create new milestones automatically #37
Conversation
Signed-off-by: Geert Eltink <dev@elt.ink>
7e13015
to
0271e46
Compare
Signed-off-by: Geert Eltink <dev@elt.ink>
Signed-off-by: Geert Eltink <dev@elt.ink>
Signed-off-by: Geert Eltink <dev@elt.ink>
Signed-off-by: Geert Eltink <dev@elt.ink>
Signed-off-by: Geert Eltink <dev@elt.ink>
Signed-off-by: Geert Eltink <dev@elt.ink>
Signed-off-by: Geert Eltink <dev@elt.ink>
Signed-off-by: Geert Eltink <dev@elt.ink>
Signed-off-by: Geert Eltink <dev@elt.ink>
Signed-off-by: Geert Eltink <dev@elt.ink>
@Ocramius ready for final review. The test repo I used is here: https://github.com/xtreamwayz/refactored-stroopwafel/milestones. All milestones there are automatically created. |
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.
Minor changes needed. Overall OK with adding this as a new step (although perhaps earlier in the release workflow?)
We kinda need to simplify the setup for end users though, so that they don't need to change their configuration all the time (new ticket, perhaps?)
examples/.github/workflows/release-on-milestone-closed-triggering-release-event.yml
Outdated
Show resolved
Hide resolved
->withAddedHeader('Content-Type', 'application/json') | ||
->withAddedHeader('User-Agent', 'Ocramius\'s minimal API V3 client') | ||
->withAddedHeader('Authorization', 'token ' . $this->apiToken); |
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.
Unrelated to this patch, but perhaps we should have a specialized PSR-17 request factory that adds these headers to the request generated by the factory? Thoughts, @Ocramius ?
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.
@weierophinney I don't think this is the responsibility of the factory, but rather of a Github API client: the fact that a token is added, and that a user-agent is added, is specific to Github's requirements, and we can decorate the request there if needed.
I would keep it as-is unless we keep adding more and more Github API interactions.
Signed-off-by: Geert Eltink <dev@elt.ink>
Signed-off-by: Geert Eltink <dev@elt.ink>
Signed-off-by: Geert Eltink <dev@elt.ink>
Signed-off-by: Geert Eltink <dev@elt.ink>
Signed-off-by: Geert Eltink <dev@elt.ink>
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.
LGTM 🚢
Running some final tests now. |
@geerteltink ping me and I'll get 1.4.0 through tonight, if all worked :-) |
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.
Minor tweaks only, but looks good!
Signed-off-by: Geert Eltink <dev@elt.ink>
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.
Reads well, I can understand what is happening! Nice work, @geerteltink !
Description
Create next milestones after a release is prepared.
Closes #26