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

Update gemspec required_ruby_version to reflect correct supported versions #1424

Closed
wants to merge 0 commits into from

Conversation

nickfloyd
Copy link
Contributor

After reviewing a recent changeset @timrogers noticed that our gemspec stated that we support Ruby 2.0 which has not been the case for some time. This PR attempts to:

  1. Correct the Ruby version inconsistency and update the supported version to 2.3
  2. Correct the unintended "change" we're the previous PR inadvertently dropped support for Ruby 2.3 and 2.4
  3. Ensures that when Ruby 2.3-2.5 are used then the correct deps are loaded.

Note: This is a correction to a possible breaking change - we'll be discussing this shortly to see what we want to do as far as versioning and releasing it goes.

Copy link
Contributor

@timrogers timrogers left a comment

Choose a reason for hiding this comment

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

The changes here all make sense to me - which isn't surprising, since I wrote a bunch of them 😂❤️

There are two questions for me:

  1. Do we treat it as a breaking change and thus a major version to make the required_ruby_version more restrictive?
  2. We are effectively dropping 2.0, 2.1 and 2.2, so should we take this as an opportunity to drop support for other EOL'd Ruby versions - 2.4 and 2.5? (especially if we are going to bump the major version anyway!)

Personally, I am in favour of a "yes" to both of those questions: #1 for good semver and good community stewardship, and #2 so we aren't stuck with the limitations of old Ruby versions on syntax and security.

Can I also propose that we change the title of the PR to something a little more descriptive, e.g. "Update gemspec required_ruby_version to reflect correct supported versions"? 👼🏻 (That title omits the Gemfile changes, but since they are in the gemspec, they only effect development and CI and not the actual gem.)

@timrogers
Copy link
Contributor

timrogers commented Jun 2, 2022

Thanks for putting together the PR and writing up the description, by the way ✨❤️

@nickfloyd nickfloyd changed the title Fixes support for Ruby 2.3 ~ 2.4 and Faraday Update gemspec required_ruby_version to reflect correct supported versions Jun 6, 2022
@nickfloyd
Copy link
Contributor Author

  • Do we treat it as a breaking change and thus a major version to make the required_ruby_version more restrictive?

Usually, I'd agree with your "yes" to this question - for this PR I'd favor moving forward with 4 as the major only because this feels like we are really "hot fixing" or patching a previous change. I might be wrong on how I feel about it though.

I also want to avoid this because currently, we do not have a potentially shippable master.

  • We are effectively dropping 2.0, 2.1 and 2.2, so should we take this as an opportunity to drop support for other EOL'd Ruby versions - 2.4 and 2.5? (especially if we are going to bump the major version anyway!)

I think those would ideal for a 5.x jump.


My thoughts on how we move forward are:

  1. Batch up all the changes that we can and merge them into the 4.x branch
  2. Ship the final 4.x release
  3. Sync master with 4.x - perhaps a nuke and a git force push on it - I know that's not a popular thing to do for folks that have master checked out but master in its current state is so far off from what is in 4.x I don't think there is any active work in master.
  4. Delete 4.x and possibly 5.x branches - we might need to look at outstanding PRs and the like to see what type of damage that would cause with previous work.
  5. Fix tooling to make sure we release from the tip of master (I don't think there are any areas where 4.x is set other than the default branch)

Thoughts?

@timrogers
Copy link
Contributor

I'm happy to do it that way and treat this as a "correction" rather than an actual change 👍

@timrogers
Copy link
Contributor

I ended up merging the first two commits of this PR in #1441 to get confidence that my changes worked in the versions I wanted to. The bit left still to merge is the required_ruby_version change.

timrogers added a commit that referenced this pull request Jul 11, 2022
Currently, the default branch for this repo is `4-stable` (not
`main` or `master`) and we have some old code which was proposed
to be the v5.x version of Octokit.rb in the `5-alpha` branch.

We now want to release a new major version of Octokit to drop
support for old Ruby versions - see #1424.

To do that, we'll delete the unused `master` branch and rename
`4-stable` to `main`.

Ahead of that change, this updates our GitHub Actions workflows
to support the name `main`. Once the rename is done, we can
remove all of the references to the old branch names.
timrogers added a commit that referenced this pull request Jul 11, 2022
Currently, the default branch for this repo is `4-stable` (not
`main` or `master`) and we have some old code which was proposed
to be the v5.x version of Octokit.rb in the `5-alpha` branch.

We now want to release a new major version of Octokit to drop
support for old Ruby versions - see #1424.

To do that, we'll delete the unused `master` branch and rename
`4-stable` to `main`.

Ahead of that change, this updates our GitHub Actions workflows
to support the name `main`. Once the rename is done, we can
remove all of the references to the old branch names.
@timrogers timrogers closed this Jul 11, 2022
@nickfloyd nickfloyd added Type: Bug Something isn't working as documented Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR and removed bug labels Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants