Skip to content

fix(ci): add publishing tags step when publishing auxiliary packages MONGOSH-1871 #2332

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

Merged
merged 5 commits into from
Jan 28, 2025

Conversation

gagik
Copy link
Contributor

@gagik gagik commented Jan 27, 2025

lerna publish doesn't automatically push tags when using from-package so we have to create and push the tags manually.

This is different from releasing mongosh as it does its own tag push already (without mongosh@ prefix):

const mongoshVersion = packages.find(
(packageConfig) => packageConfig.name === 'mongosh'
)?.version;
if (!mongoshVersion) {
throw new Error('mongosh package not found');
}
spawnSync(
'git',
['tag', '-a', mongoshVersion, '-m', mongoshVersion],
commandOptions
);
spawnSync('git', ['push', '--follow-tags'], commandOptions);

lerna publish doesn't automatically push tags when using from-package so we have to create and push the tags manually.
@@ -58,3 +58,10 @@ jobs:
npm config list
echo "Publishing packages as $(npm whoami)"
npm run publish-auxiliary

- name: "Publish tags"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

essentially same as https://github.com/mongodb-js/compass/blob/main/.github/workflows/publish-packages.yaml#L60 with one adjustment to not push tags for mongosh in this case

- name: "Publish tags"
run: |
npx lerna list -a --json | \
jq -r '.[] | select(.name != "mongosh") | .name + "@" + .version' | \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is mongosh excluded here because it's not scoped with @mongosh/? Maybe add a comment or even a test somewhere that reminds people to update this line if that ever changes

Copy link
Contributor Author

@gagik gagik Jan 27, 2025

Choose a reason for hiding this comment

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

I am excluding it here as it is the publishing auxiliary packages which would mean it'd try to publish [email protected] multiple times and make the tag meaningless.

Come to think of it, it'd be trivial to extract this to a function as well and make this distinction easier to reason about (and test/enforce) so I'll do that. It'd be a good test for the release git run also as I'm not 100% on how well it can access git.

@gagik gagik force-pushed the gagik/fix-pushing-tags branch 2 times, most recently from f54ec3f to d464fa2 Compare January 27, 2025 20:23
@@ -22,6 +23,7 @@ export async function publishMongosh(
barque: Barque,
createAndPublishDownloadCenterConfig: typeof createAndPublishDownloadCenterConfigFn,
publishToNpm: typeof publishToNpmType,
pushTags: typeof pushTagsType,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these helper functions are getting out of hand and only seem to exist for the purpose of spying/stubbing with sinon. I am keeping same structure here but think it'd be good to wrap them in a namespace or something in the future.

@gagik gagik requested a review from addaleax January 28, 2025 08:43
Includes tests and remote checking of tags.
@gagik gagik force-pushed the gagik/fix-pushing-tags branch from d464fa2 to de6927b Compare January 28, 2025 08:49
@gagik gagik force-pushed the gagik/fix-pushing-tags branch from 2e32552 to 0f85b12 Compare January 28, 2025 10:30
@gagik gagik merged commit e83f504 into main Jan 28, 2025
12 of 13 checks passed
@gagik gagik deleted the gagik/fix-pushing-tags branch January 28, 2025 11:07
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.

3 participants