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

Move examples dir into its own Go module #2310

Merged
merged 1 commit into from Mar 8, 2022

Conversation

wlynch
Copy link
Contributor

@wlynch wlynch commented Mar 2, 2022

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).

@codecov
Copy link

codecov bot commented Mar 2, 2022

Codecov Report

Merging #2310 (0c53a68) into master (2a15bc2) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a15bc2...0c53a68. Read the comment docs.

example/go.mod Outdated Show resolved Hide resolved
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).
Copy link
Collaborator

@gmlewis gmlewis left a 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.

@frioux
Copy link

frioux commented Mar 8, 2022

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 😅

@gmlewis
Copy link
Collaborator

gmlewis commented Mar 8, 2022

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. 😁

@asvoboda
Copy link
Contributor

asvoboda commented Mar 8, 2022

👍 LGTM.

@gmlewis
Copy link
Collaborator

gmlewis commented Mar 8, 2022

Thank you, @asvoboda !
Merging.

@gmlewis gmlewis changed the title Move examples into its own Go module Move examples dir into its own Go module Mar 8, 2022
@gmlewis gmlewis merged commit b4e931b into google:master Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants