-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Move examples dir into its own Go module #2310
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2310 +/- ##
=======================================
Coverage 97.84% 97.84%
=======================================
Files 115 115
Lines 10403 10403
=======================================
Hits 10179 10179
Misses 156 156
Partials 68 68 Continue to review full report at Codecov.
|
We notices that go-github has an example that references ghinstallation, which causes a circular module dependency. While this is technically fine (https://go.dev/ref/mod#minimal-version-selection should resolve it), there's probably not a strong reason to pull in ghinstallation and other example dependencies not needed by the core github package unless it is strictly needed. This just breaks the example folder into its own module so that users can import the core go-github package with the minimal set of dependencies. This should not affect any functionality. I tried to keep versions the roughly same for compatibility, though replaced the go-github dependency with the parent to try and keep examples up-to-date with HEAD (this could possibly be replaced with https://go.dev/ref/mod#workspaces later).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @wlynch !
(In the future, please don't force-push PRs for this repo, as we always squash-and-merge at the end anyway... see CONTRIBUTING.md for more details.)
LGTM.
Awaiting second LGTM from any other contributor to this repo.
Looking forward to this one getting merged. The fact that the current release expresses a dependency on an older version of itself... seems not great 😅 |
If you (or any other contributor) give me a second LGTM/Approval, @frioux , I can go ahead and merge it. 😁 |
👍 LGTM. |
Thank you, @asvoboda ! |
We (in particular shoutout to @asvoboda for noticing this in bradleyfalzon/ghinstallation#63)
noticed that go-github has an example that references ghinstallation,
which causes a circular module dependency. While this is technically
fine (https://go.dev/ref/mod#minimal-version-selection should resolve
it), there's probably not a strong reason to pull in ghinstallation and
other example dependencies not needed by the core github package unless
it is strictly needed.
This just breaks the example folder into its own module so that users
can import the core go-github package with the minimal set of
dependencies. This should not affect any functionality.
I tried to keep versions roughly the same for compatibility, though
replaced the go-github dependency with the parent to try and keep
examples up-to-date with HEAD (this could possibly be replaced with
https://go.dev/ref/mod#workspaces later).