-
Notifications
You must be signed in to change notification settings - Fork 2
Add GitHub action for header-gen build #6
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
This will publish the ml-header-generator package.
The setup-node action expects this to be at the root.
This may fail if this repo doesn't have permissions to use this package
.github/workflows/build.yml
Outdated
| - uses: microbit-foundation/npm-package-versioner-action@v1 | ||
| with: | ||
| working-directory: ./header-gen |
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.
Assuming the workflow will be to keep updating the extension to pxt bump, and to manually create a GH release from an existing tag when need to push this package, then that means the package will follow the same version as the pxt extension.
I think this is perfectly fine, just wanted to double check this is the intention and the proposed workflow.
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 think this makes sense.
|
@microbit-carlos I've attempted the merge with |
| working-directory: ./header-gen | ||
|
|
||
| - name: Update version | ||
| if: github.event_name != 'pull_request' |
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 step will also run on any pushes with changes to the header-gen source code, even if they are not meant to update the package version yet (which likely has already increased as we pxt bump the extension), is that the intention?
I assume this action only creates a commit if the version in package.json and the version in the last repo tag don't match?
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.
In this case, the action should create a published version with something like @0.1.0-[branch_name].[build_number]. It should never create any commits.
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.
But looking at the description in https://github.com/microbit-foundation/npm-package-versioner-action it sounds like if the package.json version entry needs to be updated it will do that? Does it do it without committing the change to the repo?
So, it changes package.json, builds package, pushes it to the package registry with a temporary tag like @0.1.0-[branch_name].[build_number] and doesn't commit anything? At what point it will push a package with the correct version number?
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.
sorry, the publishing is done by the npm publish step.
So, the microbit-foundation/npm-package-versioner-action action will change the package.json version entry if needed (maybe to 0.1.0-[branch_name].[build_number] as you mentioned), without committing anything and then it's up to other steps to create and/or publish the package. Is that right?
How does it decide if the version entry in package.json needs to be changed? is it if this commit is tagged AND the tag version is the same as the version in package.json?
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.
Apologies. This is ignorance on my part. We've not used this in prod yet and I naively assumed it would work in the same way as the package it's trying to replace.
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.
OK. It looks like it just changes the version in package.json without commiting anything. The udpated version number is only used for npm publish in that workflow.
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.
Great, thanks Rob!
|
@microbit-robert just a friendly ping that there are still a couple of open comments on this PR before we can merge it, which we might want to do to publish this package for (microbit-foundation/pxt-microbit-ml#5) |
Co-authored-by: Carlos Pereira Atencio <carlos@microbit.org>
microbit-carlos
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.
Looks good, thanks Rob!
|
@microbit-robert Publishing failed in: Does the access right for this repo has to be managed from the org settings? |
It's trying to publish in the wrong place. I'm guessing it's not reading the .npmrc file in working-directory. We can try moving it to the root of the project and also changing the name in the package.json from "ml-header-generator" to "@microbit-foundation/ml-header-generator". |
|
In the end it only needed the |
No description provided.