utils/github: split GitHub module

[nandahkrishna]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?
• Have you successfully run brew man locally and committed any changes?

---

The GitHub module in utils/github.rb was getting too large - beyond the 600 lines style limit - and in #10518 it was decided to temporarily relax that limit, and work on splitting the module in a follow-up PR.

This PR splits the GitHub module in the following manner:

• utils/github/api - all the "access API" and credentials methods and error classes.
• utils/github - wrapper functions to access specific API endpoints.

As a result of this PR, changes will have to be made in some files in other repositories (including Homebrew/homebrew-cask and Homebrew/homebrew-linux-dev). I'm collecting a list of changes to make and will update it here.

See #10518 (comment).

[BrewTestBot]

Review period will end on 2021-02-16 at 17:15:01 UTC.

[MikeMcQuaid]

This feels like a good split, nice work!

[MikeMcQuaid]

As a result of this PR, changes will have to be made in some files in other repositories (including Homebrew/homebrew-cask and Homebrew/homebrew-linux-dev). I'm collecting a list of changes to make and will update it here.

Is it worth adding wrapper functions for now which are odeprecated later?

[nandahkrishna]

Is it worth adding wrapper functions for now which are odeprecated later?

That sounds good, yes. Will keep things working while I open PRs to update usage in the other repositories.

[nandahkrishna]

I've added wrappers for the methods used in other files/repositories (methods in utils/github/api except the ones that print error messages) and also the error classes (included these just in case they were necessary elsewhere).

If the way I've handled this is incorrect, please let me know.

[MikeMcQuaid]

May need to deprecate these in the new minor release instead? What does @Homebrew/brew think? Also/instead: I think you can limit these to the only ones you've specifically seen used elsewhere for now?

[Bo98]

Since this whole module is private API, we can forgo the usual deprecation process.

However, since some of these are used in other Homebrew repositories, we should consider whether these should actually be public API instead. My view is private API usage in other repositories should be kept to a minimum.

[nandahkrishna]

I guess usage isn't widespread (for the methods that have been moved): I found 6 instances of open_api usage so far, outside of Homebrew/brew. These were spread across 3 of our repositories (homebrew-linux-dev, actions and homebrew-cask).

Also/instead: I think you can limit these to the only ones you've specifically seen used elsewhere for now?

Alright, seems like it might only be open_api, but I'll check again to be sure.

[MikeMcQuaid]

Since this whole module is private API, we can forgo the usual deprecation process.

If it's specified/documented as such: I agree.

However, since some of these are used in other Homebrew repositories, we should consider whether these should actually be public API instead. My view is private API usage in other repositories should be kept to a minimum.

Agreed but this isn't always feasible. Homebrew/homebrew-bundle, for example, makes extensive use of private APIs because it's too slow to do otherwise. We could make these public, of course.

Alright, seems like it might only be open_api, but I'll check again to be sure.

Cool, in that case then: yeh, just a wrapper for that seems sufficient.

[Bo98]

If it's specified/documented as such: I agree.

@api private is our documentation here. Labels it nicely here: https://rubydoc.brew.sh/GitHub.html

Agreed but this isn't always feasible.

For sure. I don't expect it to be zero.

My general thinking is: what are we doing in these repositories that we think that no one outside the Homebrew org should be replicating? For cases where there isn't a strong reason, it should be public API.

[MikeMcQuaid]

My general thinking is: what are we doing in these repositories that we think that no one outside the Homebrew org should be replicating? For cases where there isn't a strong reason, it should be public API.

Yup, that's fair. I should go and upgrade the brew bundle ones to be public.

[BrewTestBot]

Review period ended.

[nandahkrishna]

As suggested, I've retained the wrapper method only for open_api (now API.open_rest) and modified the names of methods in the API module which contained the word api.

I haven't made the GitHub module @api public but if that's preferred I'll go ahead and make the change, and remove the odeprecated for the wrapper method.