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

Bump go.mod version to v2 #545

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

pbusko
Copy link
Contributor

@pbusko pbusko commented Jul 8, 2024

Summary

To reflect the actual major version and follow the official recommendation regarding go.mod versioning

Use Cases

https://go.dev/doc/modules/version-numbers

A major version update to a number higher than v1 will also have a new module path. That’s because the module path will have the major version number appended, as in the following example:

module example.com/mymodule/v2 v2.0.0

A major version update makes this a new module with a separate history from the module’s previous version. If you’re developing modules to publish for others, see “Publishing breaking API changes” in Module release and versioning workflow.

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
  • I have added an integration test, if necessary.
  • I have reviewed the styleguide for guidance on my code quality.
  • I'm happy with the commit history on this PR (I have rebased/squashed as needed).

@pbusko pbusko requested a review from a team as a code owner July 8, 2024 08:27
@pbusko pbusko requested review from ryanmoran and TisVictress July 8, 2024 08:27
@pacostas
Copy link
Contributor

pacostas commented Jul 9, 2024

Hello @pbusko 👋 ! Thank you for your contribution :) Can you describe how consuming the versioned node-start from the node-start module will benefit the node-start module? I'm asking for a little bit of context as this change makes sense for consuming external modules, but I can not find a scenario where the same module would benefit from not consuming the latest version of itself. Also, it seems that this change is adding an extra step during development, as for consuming the latest version of the node-start you will have to make a publish first.

@pbusko
Copy link
Contributor Author

pbusko commented Jul 10, 2024

@pacostas pacostas added the semver:patch A change requiring a patch version bump label Jul 10, 2024
pacostas
pacostas previously approved these changes Jul 10, 2024
Copy link
Contributor

@pacostas pacostas left a comment

Choose a reason for hiding this comment

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

LGTM

@c0d1ngm0nk3y c0d1ngm0nk3y force-pushed the bump-go-mod branch 2 times, most recently from 4759564 to b3c63ba Compare July 23, 2024 06:39
@pacostas pacostas dismissed their stale review July 23, 2024 12:39

I need to clarify the actual benefits of this PR and in case we need to follow a versioning we need to consider strategy for that.

@c0d1ngm0nk3y
Copy link
Contributor

see discussions on paketo-buildpacks/node-engine#966

@pbusko pbusko requested a review from pacostas July 25, 2024 08:56
@pacostas
Copy link
Contributor

Related issue paketo-buildpacks/nodejs#913

@mhdawson
Copy link
Member

mhdawson commented Aug 8, 2024

Based on discussion in paketo-buildpacks/nodejs#913 we'll land these add hock for now. @pbusko can you rebase and then I'll land.

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM but a rebase will be needed.

@pbusko
Copy link
Contributor Author

pbusko commented Aug 8, 2024

Thank you @mhdawson, the PR has been rebased

Copy link
Contributor

@pacostas pacostas left a comment

Choose a reason for hiding this comment

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

@mhdawson
Copy link
Member

mhdawson commented Aug 9, 2024

@pbusko unfortunately seems to need another rebase. Will enable auto-merge so it should land as soon as the tests pass. I had started them yesterday but seems there was some change between then and now.

@mhdawson mhdawson enabled auto-merge (squash) August 9, 2024 13:35
auto-merge was automatically disabled August 9, 2024 23:20

Head branch was pushed to by a user without write access

@pacostas
Copy link
Contributor

@pbusko can you rebase again?

@pacostas pacostas enabled auto-merge (squash) August 13, 2024 08:10
@pacostas
Copy link
Contributor

@pbusko can you rebase once more? I thought the auto merge was enabled and in the mid-time another PR automatically landed. sorry :)

Co-authored-by: Ralf Pannemans <[email protected]>
auto-merge was automatically disabled August 13, 2024 08:30

Head branch was pushed to by a user without write access

@pacostas pacostas enabled auto-merge (squash) August 13, 2024 08:31
@pacostas pacostas requested review from pacostas and removed request for pacostas August 13, 2024 08:32
@pbusko
Copy link
Contributor Author

pbusko commented Aug 13, 2024

@pacostas could you please re-trigger the integration tests? Yet another failure due to the network issues while downloading nodejs package.

@pacostas pacostas merged commit c896363 into paketo-buildpacks:main Aug 13, 2024
10 checks passed
@pbusko pbusko deleted the bump-go-mod branch August 13, 2024 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:patch A change requiring a patch version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants