-
Notifications
You must be signed in to change notification settings - Fork 325
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
fix(release-automation): Release Please Using Manifests #1149
Conversation
b5d426b
to
986d4e4
Compare
3dd245b
to
d337c7e
Compare
@@ -100,12 +100,8 @@ jobs: | |||
steps: | |||
- uses: google-github-actions/[email protected] | |||
with: | |||
release-type: node | |||
package-name: ipfs-companion | |||
command: manifest-pr |
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.
now it would look for config and manifest files.
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.
can you expand on how this fixes things?
"bootstrap-sha": "65fe66cc283f1ac8c89638034e7db006e1f53791", | ||
"last-release-sha": "7td2b9838885b3adf52e78ddd23ac01cb819e631", |
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 the sha of the last release and sha of the release we want to start counting changes from.
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.
will these be updated automatically?
"packages": { | ||
".": { | ||
"release-type": "node", | ||
"plugins": ["node-workspace"], | ||
"package-name": "ipfs-companion", | ||
"release-as": "2.22.0" | ||
} | ||
} |
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.
since manifests are designed to be setup for monorepo, the one package config looks like this.
"extra-files": [ | ||
{ | ||
"type": "json", | ||
"path": "add-on/manifest.common.json", | ||
"jsonpath": "$.version" | ||
} | ||
], |
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.
and these cannot be associated with the package, because 🤷🏽
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 some questions
@@ -100,12 +100,8 @@ jobs: | |||
steps: | |||
- uses: google-github-actions/[email protected] | |||
with: | |||
release-type: node | |||
package-name: ipfs-companion | |||
command: manifest-pr |
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.
can you expand on how this fixes things?
@@ -0,0 +1,3 @@ | |||
{ | |||
".": "2.21.0" |
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.
is this updated automatically?
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.
answered your questions in the PR description 🔝
"bootstrap-sha": "65fe66cc283f1ac8c89638034e7db006e1f53791", | ||
"last-release-sha": "7td2b9838885b3adf52e78ddd23ac01cb819e631", |
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.
will these be updated automatically?
"release-type": "node", | ||
"plugins": ["node-workspace"], | ||
"package-name": "ipfs-companion", | ||
"release-as": "2.22.0" |
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.
is this updated automatically?
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.
answered your questions in the PR description 🔝
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
In this PR:
main
)Addressing @SgtPooki's questions:
How does
manifest-pr
fixes things.because
release-please
cannot decide how they want to deal with things,manifest[-pr]
tells release-please to look for two files in the root of the base branch..release-please-manifest.json
andrelease-please-config.js
we can pass config as yaml args, but that does not control everything likejsonUpdater
that we need to update arbitrary json e.g.manifest.common.json
. Since the feature parity is lacking, it's assumed (even by them) that manifest is the better way of doing this.Regarded hard-coded sha, will those be updated automatically?
No, but since is the first time a PR is being generated by release please, it needs to be manually boot-strapped. I'll be creating a follow-up PR after the first one is merged. That will ensure we have all the required files it needs in the base branch
main
. The demo PR contains all the files that will be updated automatically.