Skip to content
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

docs: add development docs #1

Merged
merged 7 commits into from
Oct 4, 2023
Merged

docs: add development docs #1

merged 7 commits into from
Oct 4, 2023

Conversation

robertsLando
Copy link
Member

No description provided.

@robertsLando
Copy link
Member Author

robertsLando commented Oct 3, 2023

cc @baparham if you could kindly tell me your opinion about the steps I listed in this document 🙏🏼

DEVELOPMENT.md Outdated
- Once the new patch is merged we can trigger the `.github/workflows/build-all.yml` workflow to build all the binaries for all the platforms.
- If this is a minor/major release we need to check the checkbox of the workflow to create a draft release.
- Once the actions will end, copy the sha256 checksums printed at the end of the release body and update`lib/expected-shas.json` file with the correct values.
- If this is not a new release, download the new nodejs binaries and upload them as assets to the active release.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there was one part of the build-all workflow that automatically uploaded the binaries to the latest release or something, but I'd have to dig into the workflow to find out

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah when the checkbox Create new draft release of the build-all workflow is not checked it will upload releases to the latest one, problem is that this is a fork and there is no releases so I had to create a fake one first and then I have deleted the "fake" release and renamed the untagged one created by the workflow with the correct changelog. All artifacts have been built successfully 🎉

DEVELOPMENT.md Outdated
- In order to support a new nodejs version we need to create/update the patch in `patches` folder and also `patches/patches.json` file.
- Once the new patch is merged we can trigger the `.github/workflows/build-all.yml` workflow to build all the binaries for all the platforms.
- If this is a minor/major release we need to check the checkbox of the workflow to create a draft release.
- Once the actions will end, copy the sha256 checksums printed at the end of the release body and update`lib/expected-shas.json` file with the correct values.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that the workflow itself posts a summary with the SHAs so they are no longer in the (mutable) release, but rather attached to the workflow that is seemingly immutable. I don't know.

Copy link
Member Author

@robertsLando robertsLando Oct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this, let me check

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see there is a script in package.json named updateExpected but that is never called in the buildall process. I think that should be manually run and by passing the generated sha in the workflow release:

.option('input', { alias: 'i', default: 'shas.txt', type: 'string' })

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok seems that the sha txt of each version in the release are not used anymore, we actually use the sha shipped with the package instead. I updated the docs with the process to create/update them

@baparham
Copy link

baparham commented Oct 3, 2023

I think it's worth highlighting that we like to check a patch by building the binaries as well, since sometimes the patches don't build even if they apply cleanly

@robertsLando
Copy link
Member Author

robertsLando commented Oct 4, 2023

I think it's worth highlighting that we like to check a patch by building the binaries as well, since sometimes the patches don't build even if they apply cleanly

Do you mean this?

pkg-fetch/lib/bin.ts

Lines 54 to 57 in 549db84

if (test) {
await verify(local);
}
}

I added to readme a mention to it, thanks 🙏🏼

@robertsLando robertsLando merged commit cc7de4b into main Oct 4, 2023
5 checks passed
@robertsLando robertsLando deleted the dev-docs branch October 4, 2023 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants