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

maint: Use go mod replace to pull gopacket dependency #183

Merged
merged 6 commits into from
Sep 15, 2023

Conversation

MikeGoldsmith
Copy link
Contributor

@MikeGoldsmith MikeGoldsmith commented Sep 14, 2023

Which problem is this PR solving?

Updates the agent's gopacket usage to default to gopacket/gopacket and then swap in our fork honeycombio/gopacket with a go mod replace directive. This allows the fork to remain cleaner because we won't need to rename the package namespace, easier to manage by referencing commit SHAs instead of our own release tags, and provides a place from which we can open upstream contributions.

Short description of the changes

  • Update imports of github.com/honeycombio/gopacket (fork) with github.com/gopacket/gopacket (upstream)
  • Add replace directive to go.mod that swaps in honeycombio/gopacket (fork)
  • Document how to update which "version" of our fork we depend on.

How to verify that this has the expected result

Dependencies are cleaner with go.mod depending on official packages then swap in the fork that the agent can experiment with.

@MikeGoldsmith MikeGoldsmith added the type: maintenance The necessary chores to keep the dust off. label Sep 14, 2023
@MikeGoldsmith MikeGoldsmith self-assigned this Sep 14, 2023
@MikeGoldsmith MikeGoldsmith requested a review from a team September 14, 2023 22:59
We maintain a fork of [gopacket/gopacket](https://github.com/gopacket/gopacket) as [honeycombio/gopacket](https://github.com/honeycombio/gopacket).
The agent is configured to use the official gopacket repo as part of it's main dependency chain and import paths.
The honeycomb fork is swaped in using a replace directive in go.mod.
This allows the fork to remain cleaner, easier to manage and makes it easier to provide upstream contributions.
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻 🎉 👍🏻 🎉 👍🏻 🎉
tenor-60851628

Copy link
Member

@robbkidd robbkidd left a comment

Choose a reason for hiding this comment

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

Fork yeah!

@robbkidd
Copy link
Member

@MikeGoldsmith I've got some edits to DEVELOPING incoming for us to consider.

- Appease markdown linter by turning the steps into bullets
  and most of the text and code blocks into paragraphs
  between them.
- Fix some typos.
Copy link
Contributor

@JamieDanielson JamieDanielson left a comment

Choose a reason for hiding this comment

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

couple small recommendations based on my initial read, to help with clarity

DEVELOPING.md Show resolved Hide resolved
DEVELOPING.md Show resolved Hide resolved
DEVELOPING.md Outdated Show resolved Hide resolved
DEVELOPING.md Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
@robbkidd robbkidd merged commit 931448c into main Sep 15, 2023
4 checks passed
@robbkidd robbkidd deleted the mike/hny-gopacket branch September 15, 2023 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: maintenance The necessary chores to keep the dust off.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants