-
Notifications
You must be signed in to change notification settings - Fork 10
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
Versioning guide #31
Comments
@Clever/engineers your thoughts? |
|
I think it'd be great to revive this. Frustrating to merge to master and come back after expected build time only to find NPM/Docker deploy failed due to version collisions. |
+1 to documenting this and having a process. In order to make this more robust/automated, I'm highly in favor of Drone auto-bumping the patch version every time merge to master happens. It's safer now that we run Docker images, because consumers of the NPM package won't easily be broken by an (mistakenly) incompatible patch version.
|
Benefit is mitigating confusion/broken builds |
But auto-bumping the patch version would not be following semver, which seems like a significant downside to me |
can you clarify why not following? (e.g. some changes shouldn't be patches. OR might publish non-patch as patch) |
Another option: change drone logic. if NPM publish fails, don't fail the build. still build docker image |
Not all changes are patches - some are minor and some are major, so auto-patching is going to break semver |
My thought would be that eng is responsible for specifically changing the package.json for minor/major updates ONLY, before merging. But true, if it's automatic, someone will probably forget and eventually a minor/major change will mistakenly get automatically published as a patch version. |
Yeah I agree with @azylman 's sentiment against auto touching the version. |
FWIW, I prefer doing a version bump in the PR and not post PR. I don't it should have any particular side effects but not sure. Reason being that it seems natural include version bumps with any change. Also ensures that we can review that the version is being bumped correctly. |
Should cover:
npm version
)Questions:
The text was updated successfully, but these errors were encountered: