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

Support more minitar versions & prep for r10k 5.0 #major #1408

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

justinstoller
Copy link
Member

I would consider moving from requiring Ruby 2.6 to Ruby 3.1 to be a major breaking change. One we should totally do, but one I think requires a major version bump. We should see what else in the PR backlog we want to get in for such a release.

Also, this allows us to support minitar version > 0.9, to smooth transitions for people who require both r10k and puppet in their Gemfiles/gemspecs.

This changes us from requiring at least minitar 1.0 and instead goes to
the previous minimum verion (0.9), but instead of using a "~>" which
forbids going to a new major release, ie is equivalent to [">= 0.9", "< 1.0"]
we simply require above the previous version, not caring if we take up a
new major release. This allows us to be installed in the same version
Ruby bundle as Puppet (which currently does not support minitar >= 1.0).

Minitar has supported the "new" `::Minitar` constant since before 0.9,
so there are no code changes required.
@justinstoller justinstoller requested review from a team as code owners October 30, 2024 18:31
@justinstoller
Copy link
Member Author

Do not merge yet. I definitely want @bastelfreak 's feedback first!

- Add Ruby 3.3 to CI [#1403](https://github.com/puppetlabs/r10k/pull/1403)
- Require Ruby 3.1 [#1402](https://github.com/puppetlabs/r10k/pull/1402)
- Support minitar gem > 1.0. [#1407](https://github.com/puppetlabs/r10k/pull/1407)
Copy link
Member Author

Choose a reason for hiding this comment

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

shoot, this PR ended up being 1408, I guess I missed 1407.

Copy link
Contributor

Choose a reason for hiding this comment

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

To prevent this in the future I raised #1404 some time ago :D

@justinstoller justinstoller mentioned this pull request Oct 30, 2024
r10k.gemspec Outdated Show resolved Hide resolved
r10k.gemspec Outdated Show resolved Hide resolved
bastelfreak added a commit to bastelfreak/ra10ke that referenced this pull request Oct 30, 2024
@bastelfreak
Copy link
Contributor

bastelfreak commented Oct 30, 2024

I gave this a quick test on ruby 3.1 and newer + puppet_forge 6 / minitar 1.0.2 in our downstream tool ra10ke: voxpupuli/ra10ke#110 . That looks fine.

Edit:
interestingly it passes also with puppet_forge 5.0.4 / minitar 0.12.1: https://github.com/voxpupuli/ra10ke/actions/runs/11599737478/job/32298544777?pr=111 . But I don't know if the tests actually hit the code paths that use minitar 🤔

bastelfreak added a commit to bastelfreak/ra10ke that referenced this pull request Oct 30, 2024
bastelfreak added a commit to bastelfreak/ra10ke that referenced this pull request Oct 30, 2024
@justinstoller
Copy link
Member Author

How do you feel about making this a 5.0 release, @bastelfreak ? Are there any other PRs you'd like to see merged for a 5.0 release?

@bastelfreak
Copy link
Contributor

Can you take a look at #1407 about the gettext integration? In case that's dead we could rip it out now as well.

@justinstoller
Copy link
Member Author

I looked into removing gettext and unfortunately we can't at the moment. I'm still open to other PRs getting in for 5.

I am unfortunately taking Friday and next week off. It feels rude to push out this release another two weeks, but it feels more irresponsible to push out a release and then leave for a week. While I'm gone please let me know what you think of existing PRs and open any other small changes you'd like to go out!

@bastelfreak
Copy link
Contributor

I totally agree that we should not rush major releases. I don't have anything else in my mind for the next release right now, but I will think about it.

@justinstoller
Copy link
Member Author

Hey Tim,

Seems like there was changes while I was on break! I've gotten approval to go ahead and keep releasing R10K though. Have you thought about additional changes to go in?

@bastelfreak
Copy link
Contributor

hi,
no, I think it's ready to go!

@nabertrand
Copy link
Contributor

@justinstoller @bastelfreak what about #1371? It would be good to fix the 'bad object ref' issue and the change could be considered breaking.

@nabertrand
Copy link
Contributor

I created #1410 following @bastelfreak's suggestion to make a separate PR for the pruning issue. Let me know if there's anything else I can do.

@justinstoller
Copy link
Member Author

See #1411 for the non-release part of this PR. I'm going to merge that while we figure out if there's a simple fix to #1399 (comment) or if it's something we shouldn't block the release on.

@nabertrand
Copy link
Contributor

@justinstoller I won't have time to look at #1399 until after the US holiday break just so you're aware of the timing.

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