Add support for lazy-loading by requiring `octokit/lazy`, rather than just `octokit`

[timrogers]

#1351 introduced support for "lazy loading" in the library rather than loading everything upfront. @gmcgibbon ran some benchmarks and found that this approach shaved 400ms off their application startup.

This was a nice idea - however, it ended up unexpected being a breaking change because users were relying on all classes and modules in Octokit being loaded upfront (e.g. so rescue Octokit::TooManyRequests works). We ended up reverting the change in #1428.

This introduces new, opt-in lazy-loading. All you need to do to take advantage of it is require 'octokit/lazy' instead of requiring plan old 'octokit'.

In our CI run, we test the gem with both methods of requiring, which should help to avoid any unexpected surprises when we release changes.

[timrogers]

@gmcgibbon How do you feel about this approach? It allows you to opt-in for faster boot times, whilst not ending up being a breaking change in certain cases.

[timrogers]

Thanks for taking a look at this so quickly ❤️

You're right - it would be possible to use autoload declarations throughout, in place of the current requires, and then everything can be lazy loaded seamlessly. I believe that can even work where one file provides multiple modules - we don't necessarily have to split them.

However, even with that in mind, I'd still argue that it might be better for lazy-loading to be optional. Some users would likely prefer the "work" to happen when their app boots, rather than adding "warmup time" to their first request (as an example).

What do you think? If we do want to provide optional lazy loading, then we could merge this version and then improve the lazy loading process as a follow-up.

[gmcgibbon]

What do you think? If we do want to provide optional lazy loading, then we could merge this version and then improve the lazy loading process as a follow-up.

You could also just use Zeitwerk and support optional eager loading and autoloading like Rails does to be more flexible. However, I think it would be useful to look at benchmarks or profiles of the load time added by autoloading to a first request before deciding on anything.

As an intermediary step, we can prep the repo for autoload support by organizing the modules and files like I mentioned because Zeitwerk also requires this (as does Rails).

[timrogers]

I did consider Zeitwerk, but I was a little wary of adding runtime dependencies and I wasn't sure how it worked in the gem context.

I'm happy to pause this PR, do some more digging and come back.

I do think this solution is a good start, but it might be worth skipping straight to something better.

Thank you for your feedback and for the original PR - I hope we can bring these performance improvements out soon.

[github-actions[bot]]

👋 Hey Friends, this pull request has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!