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

[build][bug] JS version string does not match the NPM version #8370

Closed
hydrosquall opened this issue Aug 19, 2022 · 10 comments
Closed

[build][bug] JS version string does not match the NPM version #8370

hydrosquall opened this issue Aug 19, 2022 · 10 comments
Labels
Altair Issue that is blocking Altair Bug 🐛 P1 Critical -- to fix ASAP

Comments

@hydrosquall
Copy link
Member

hydrosquall commented Aug 19, 2022

Reproduction case

image

  • Import vega-lite@5.5.0, import the version string. Note that 5.5.0 and 5.2.0 do not match.

Demo notebook: https://observablehq.com/d/175e42ebdc73221d

Leads

Stable is updated

However, Next is not

I think this is a result of the bug and/or missing workflow step I reported here: intuit/auto#2047

The fix might involve options 1 or 2 in the issue linked above. 1 is probably safer but creates extra commits, 2 is OK as long as we are comfortable with having an admin hard reset next each time a major release is cut.

@domoritz
Copy link
Member

domoritz commented Aug 19, 2022

Huh, I thought releases happen from the stable branch, not the next branch.

Either way, I would prefer extra commits than having to do a hard reset of main. We would have to do the reset not just for major releases but any releases, right?

@hydrosquall can you help fix this issue?

@hydrosquall
Copy link
Member Author

hydrosquall commented Aug 22, 2022

Releases happen from the stable branch, not the next branch.

The release is created when next merges into stable

Image 2022-08-22 at 7 15 26 PM

Unfortunately, it looks like each time that happens, whatever version was present on next overwrites whatever was on stable. The "source of truth" for the version to bump from isn't the package.json, it's the last stable version published to Github / NPM. I think that's why the autogenerated version is bumped correctly. (see the jump from 5.2 to 5.5 here)

0a2b57f

We would have to do the reset not just for major releases but any releases, right?

For any non-canary release, yea.

can you help fix this issue?

I'm not sure about the best approach for a long term fix ( this is really something we should solve for at the auto level intuit/auto#2047 ), but I can think about what a decent short term solution could be within the context of Vega. Specifically, this means could mean

  • Bumping next's package.json to 5.6 , and merging Release 5.6.0 #8368 immediately after or
  • Fiddling around with the publish step to see if we can either get bumping to happen sooner, or publishing to happen later after the bot auto increments

I think 1 is a good enough solution to start with, since incrementing Vega minor versions isn't too frequent of an event.

@domoritz
Copy link
Member

This issue is causing problems in the Vega editor as well where the displayed version is not correct.

I think we need to find a fix soon or should revert back to making releases manually.

@hydrosquall do you have an idea for

Fiddling around with the publish step to see if we can either get bumping to happen sooner, or publishing to happen later after the bot auto incremenets

@hydrosquall
Copy link
Member Author

Hey @domoritz sorry for the delay here - I haven't been actively working on the intuit ecosystem as of late.

I took a closer look at the shipit docs, and have two courses of action in mind. The first comes from the

  1. To update the latest stable version simply merge your pre-release branch into your baseBranch.

I think this is a typo because what we actually need the reverse: getting the version that is sitting in stable to show back up in next.

I believe it's proposing merging stable back into next every time that we do a release. I think that will let us reuse the appropriate version.

I also just noticed there's a flag we could try that may help, but I'm not eager to spend too much more time debugging more CLI options. https://intuit.github.io/auto/docs/generated/npm#commitnextversion

  1. The other option is to let auto stick around for the purposes of deploying canary branches in PRs, but to not use it for doing official releases until there's time to either do an official auto solution, or switch to an automatic release package (like https://www.npmjs.com/package/release-it ) which is more actively maintained and doesn't have this problem. Neither of the original maintainers of auto still work at intuit, which has probably made it more difficult to find the time to keep maintaining it

If we do this, we could stop doing merges to the stable branch, and try to release only from next. You could trigger it based on applying certain tags to the PR, or run the releases from a local laptop- depends on how manual you want to go back to.

If we just remove the "version bumping "part of auto, we could switch to using a simpler auto command which creates a github release for whatever version tag we apply manually when we do want to prepare a release

https://intuit.github.io/auto/docs/generated/release

And then in our workflow that responds to the creation of releases

https://github.com/vega/vega-lite/blob/next/.github/workflows/release-docs-and-schema.yml#L8

we could either make this workflow just publish to NPM but without trusting it to do any version bumping or changelog creation (as an alternative to publishing from someone's laptop).

@domoritz
Copy link
Member

If we do this, we could stop doing merges to the stable branch, and try to release only from next. You could trigger it based on applying certain tags to the PR, or run the releases from a local laptop- depends on how manual you want to go back to.

That seems like a good idea. We could make a release whenever there is a v* tag, which can easily be created with yarn version.

If we just remove the "version bumping "part of auto, we could switch to using a simpler auto command which creates a github release for whatever version tag we apply manually when we do want to prepare a release

I'm not a huge fan of that approach since there are a few steps involved in making a release that I would to run on the CI if possible.

@hydrosquall
Copy link
Member Author

Catching up: to confirm, are we both on board with moving forward with this ("plan 2"), meaning:

let auto stick around for the purposes of deploying canary branches in PRs, but to not use it for doing official releases
we could stop doing merges to the stable branch, and try to release only from next. You could trigger it based on applying a v* tag to the PR

I want to make sure I understand the comment (below) before doing anything, since moving ahead with "plan 2" implicitly means auto will no longer be responsible for modifying the version of the non-canary releases.

I'm not a huge fan of that approach since there are a few steps involved in making a release that I would to run on the CI if possible.

@domoritz domoritz added the Altair Issue that is blocking Altair label Feb 3, 2023
@domoritz
Copy link
Member

I'm okay with moving forward as planned.

For more long term, I would prefer switching away from auto since it doesn't work well and isn't really maintained anymore. I wonder how hard it would be to switch to release-it.

@domoritz
Copy link
Member

Switched to release-it in 5.6.1. We can bring back the canary releases in a follow up.

The version should be correct now.

@hydrosquall
Copy link
Member Author

@domoritz thanks for taking care of fixing this! The canary releases in auto were convenient, but not worth the confusion cost created by not having the package version updated correctly.

Good to know that the switch to release-it was smooth + that it's a more actively maintained package. I'll keep it in mind for future projects.

@domoritz
Copy link
Member

Thanks for all you help on this.

I think we could still have canary releases with release it but I'd need to think a bit about when to trigger them and how to not make GitHub releases. But I have other issues to work on for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Altair Issue that is blocking Altair Bug 🐛 P1 Critical -- to fix ASAP
Projects
None yet
Development

No branches or pull requests

2 participants