-
Notifications
You must be signed in to change notification settings - Fork 26
feat(ci/cd): Add releaser integration #99
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
.github/workflows/release.yml
Outdated
| permissions: | ||
| contents: write | ||
| actions: write | ||
| id-token: write |
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.
Hmm, you don't need id-token for PHP. Might need to mention on the documentation that this is only needed when using OIDC deployment - which I don't think makes sense for PHP
.github/workflows/release.yml
Outdated
| run: | | ||
| current_version="${{ steps.bump-version.outputs.current_version }}" | ||
| new_version="${{ steps.bump-version.outputs.new_version }}" | ||
| echo -e "## $new_version\n\n* [Full Changelog](https://github.com/PostHog/posthog-php/compare/v${current_version}...v${new_version})\n\n$(cat CHANGELOG.md)" > CHANGELOG.md |
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.
looks like it is currently called History.md
do we need to add a CHANGELOG.md with the History.md contents
point a link to CHANGELOG.md in the History.md
??
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.
also nit that it'd be good to have the day's date there for the future traveller
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.
Good idea! Added it to the generation flow and will backport it to the posthog-go SDK as well 🙌
pauldambra
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.
i've been removed from these changes but this is easy to change if needed so 🚢 🚢 🚢 🚢 🚢
(NB i think there needs to be some CHANGELOG creation)
rafaeelaudibert
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.
It's interesting how this is just pushing a tag rather than publishing somewhere, but lgtm! Left a minor nit but don't have to fix it here
README.md
Outdated
|
|
||
| 1. **Create your PR** with the changes you want to release | ||
| 2. **Add the `release` label** to the PR | ||
| 3. **Add a version bump label** that should be either `patch`, `minor` or `major` |
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 wrong, your action is expecting bump-patch, bump-minor or bump-major. Can you make them match?
|
Addressed both of your comments! I copied this file from the Should now be fixed!
|
rafaeelaudibert
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.
LGTM! Check the SDK Doctor code on posthog/posthog to make sure we didn't depend on History.md. IIRC we're using releases so it should be fine, but double-check it please!
|
Ah good shout! TIL that there is a connection between the 2! Here we go! PostHog/posthog#44560 |
Adds support for the
Releaser (posthog-php)integration as documented over here: https://posthog.com/handbook/engineering/sdks/releases.