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.

Sign up for GitHub

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

Jump to bottom

Bump version with npm script #800

Closed
wants to merge 1 commit into from
Closed

Bump version with npm script #800

wants to merge 1 commit into from

Conversation

pgcalixto
Copy link
Contributor

@pgcalixto pgcalixto commented Oct 4, 2020 •
edited

This is a suggestion to resolve #799.

This is not necessarily intended to be merged. It was created mainly as a suggestion for further discussion and improvements.

@pgcalixto
Bump version with npm script
26a0f3c 
The npm `version` script bumps the version on both package.json and
package-lock.json, which are currently not matching.
Also, it creates a new bump commit and tags it.
@pgcalixto pgcalixto marked this pull request as draft October 5, 2020 00:32
@pgcalixto
Copy link
Contributor Author

Changed it to draft since it is more of a suggestion up for discussion than a PR ready to merge.

@GillesDebunne
Copy link
Collaborator

GillesDebunne commented Oct 5, 2020 •
edited

I believe the bump version process is manual. It requires writing a summarized changelog, as well as deciding between a minor and a patch release, and updating package.json accordingly.

I think VERSION should be a hardcoded constant exported in luxon.js along with the other objects. That would add an other manual step when bumping version, but it is not that common.

script/release should however ensure the consistency, using require(../package.json).version as a reference, and checking it matches with package-lock and VERSION. It should abort if that is not the case, and could provide hints on the commands needed to update the latest commit accordingly (applying those automatically seems too bold).

What do you think?

@pgcalixto pgcalixto mentioned this pull request Oct 7, 2020
Merged
@pgcalixto
Copy link
Contributor Author

I agree with the hardcoded VERSION, while making script/release ensure the consistency.

The approach you suggested makes total sense. I will try to develop this approach in the other PR (#794), then I'll close this one.

pgcalixto added a commit to pgcalixto/luxon that referenced this pull request Oct 8, 2020
@pgcalixto
Revert "Export VERSION information"
3325dff 
This reverts commit 972e157.

@GillesDebunne and I agreed on a different approach, which is based
on a hardcoded VERSION export, while maintaining a logic to ensure
that the VERSION variable matches the package version.

See moment#800 (comment).
pgcalixto added a commit to pgcalixto/luxon that referenced this pull request Oct 8, 2020
@pgcalixto
Export hardcoded VERSION variable
ffd2952 
For a less bold approach, the exported VERSION variable is now
hardcoded. Meanwhile, the `scripts/tag` script was improved to
ensure that all versions match.

See moment#800 (comment).

Resolves moment#747 and moment#799.
pgcalixto added a commit to pgcalixto/luxon that referenced this pull request Oct 8, 2020
@pgcalixto
Export hardcoded VERSION variable
2a14727 
For a less bold approach, the exported VERSION variable is now
hardcoded. Meanwhile, the `scripts/tag` script was improved to
ensure that all versions match.

See moment#800 (comment).

Resolves moment#747 and resolves moment#799.
@pgcalixto pgcalixto mentioned this pull request Oct 8, 2020
Merged
@pgcalixto
Copy link
Contributor Author

Closing this as it is being reworked on #794.

@pgcalixto pgcalixto closed this Oct 18, 2020
@pgcalixto pgcalixto deleted the match-package-version-for-new-tags branch October 18, 2020 19:09
icambron pushed a commit that referenced this pull request Nov 9, 2020
@pgcalixto
Export VERSION information (#794)
343978d 
* Export VERSION information

As pointed out by @networkimprov, it is a good practice to tell the
package version by exporting a constant or having a comment to
indicate it. Therefere, the version info is exported now.

Resolves #747.

* Revert "Export VERSION information"

This reverts commit 972e157.

@GillesDebunne and I agreed on a different approach, which is based
on a hardcoded VERSION export, while maintaining a logic to ensure
that the VERSION variable matches the package version.

See #800 (comment).

* Export hardcoded VERSION variable

For a less bold approach, the exported VERSION variable is now
hardcoded. Meanwhile, the `scripts/tag` script was improved to
ensure that all versions match.

See #800 (comment).

Resolves #747 and resolves #799.

* Move version validation to dedicated script

Move the version validation to a dedicated script, so that it can
be called both from release and tag scripts, which are scripts that
should validate the version before proceeding.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

package-lock.json's version does not match package.json's
2 participants
@pgcalixto @GillesDebunne