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

Allow autoloading Octokit::Repository #1420

Conversation

waiting-for-dev
Copy link

@waiting-for-dev waiting-for-dev commented May 27, 2022

Hello!

Since dd5612e, it's not possible to reference Octokit::Repository when only lib/octokit.rb file has been required.

Steps to reproduce:

require 'octokit'
Octokit::Repository # => NameError

This is hitting as at Solidus, where our extensions try to directly reference Octokit::Repository as part of auto discovering the repository to generate a Changelog from. See https://github.com/solidusio/solidus_dev_support/blob/0764caad00f1b755d2cfdeef15762d85d5f58a01/lib/solidus_dev_support/rake_tasks.rb#L81. We found the problem since the last release yesterday.

Thanks for your work and, please, don't hesitate to ask us whatever you might need.

Since dd5612e, it's not possible to
reference `Octokit::Repository` when only `lib/octokit.rb` file has been
required.

Steps to reproduce:

> require 'octokit'
> Octokit::Repository # => NameError
Copy link
Contributor

@etiennebarrie etiennebarrie left a comment

Choose a reason for hiding this comment

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

I'm having the same issue, this PR will fix it, and a quick workaround for now is to change the Gemfile to:

gem "octokit", require: ["octokit", "octokit/repository"]

or otherwise require "octokit/repository" after the require "octokit".

@nickfloyd
Copy link
Contributor

Hey @waiting-for-dev and @etiennebarrie,

We jumped the gun a bit on auto loading - apologies for the trouble. The known issue has been documented in the release notes. I've added the workaround from this PR as well to those notes. You can also read more about all of this here.

My plan is to release this shortly. So that we can get things back to normal and start iterating on an opt-in/alternative solution for autoloading.

Again, apologies for the trouble - I'll update you once the change has been shipped.

@nickfloyd nickfloyd self-assigned this Jun 6, 2022
@nickfloyd
Copy link
Contributor

I'll be closing this issue given that it has been fixed in release v4.24.0. If you feel like this needs more discussion or that the release did not fix things please feel free to reopen this and tag @timrogers and @nickfloyd and we'll work to get things in a tidy state.

Thank you @waiting-for-dev & @etiennebarrie for your help on this. Again, apologies for the trouble.

@nickfloyd nickfloyd closed this Jun 6, 2022
@waiting-for-dev
Copy link
Author

Thanks, @nickfloyd, for your action on this. Your plan sounds good and, again, thanks for your work.

@nickfloyd nickfloyd added Type: Breaking change Used to note any change that requires a major version bump Type: Bug Something isn't working as documented and removed breaking change labels Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Breaking change Used to note any change that requires a major version bump Type: Bug Something isn't working as documented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants