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

dependency: copy external tap-go package as an internal package #766

Closed
wants to merge 1 commit into from
Closed

Conversation

fmuyassarov
Copy link

This PR fixes subset of the dependency issues described in #758.

Move/copy external tap-go package from github.com/mndrix/tap-go to be in tree package.
github.com/mndrix/tap-go is no longer maintained and it is currently archived. Also, bump
gopkg.in/yaml.v2 to gopkg.in/yaml.v3.

@fmuyassarov fmuyassarov requested a review from a team as a code owner March 29, 2023 08:49
@fmuyassarov
Copy link
Author

cc @klihub

go.mod Outdated Show resolved Hide resolved
Move/copy external tap-go package from github.com/mndrix/tap-go to
be in tree package. github.com/mndrix/tap-go is no longer maintained
and it is currently archived.
Also, bump gopkg.in/yaml.v2 to gopkg.in/yaml.v3.

Signed-off-by: Muyassarov, Feruzjon <[email protected]>
@fmuyassarov fmuyassarov requested a review from klihub March 29, 2023 12:29
Copy link
Contributor

@klihub klihub left a comment

Choose a reason for hiding this comment

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

LGTM.

@fmuyassarov
Copy link
Author

ping @mrunalp @kolyshkin

@thaJeztah
Copy link
Member

(sorry for drive by comment)

Instead of a straight copy/paste of the code, perhaps it would be good to migrate the code including history (see moby/sys#121 for an example)

Perhaps also separating the integration commits from the follow ups (formatting build tags, upgrading yaml dependency)

@fmuyassarov fmuyassarov closed this May 4, 2023
@fmuyassarov fmuyassarov deleted the tap-go branch May 4, 2023 08:36
@fmuyassarov fmuyassarov restored the tap-go branch May 4, 2023 08:37
@fmuyassarov fmuyassarov deleted the tap-go branch May 4, 2023 08:37
@fmuyassarov
Copy link
Author

@thaJeztah sorry for my late response on this, I was on PTO.
I will update my patch as you suggested.

@fmuyassarov fmuyassarov restored the tap-go branch May 4, 2023 08:39
@thaJeztah
Copy link
Member

@fmuyassarov no worries!

Also feel free to "ping" me if you need help with those instructions; I know it can be a bit of a pain to do (and make sure you use temporary repositories on your local machine), but I think it's good to a) preserve history (which can be relevant), and b) preserve the original commits to accredit the authors of those commits.

@fmuyassarov
Copy link
Author

@fmuyassarov no worries!

Also feel free to "ping" me if you need help with those instructions; I know it can be a bit of a pain to do (and make sure you use temporary repositories on your local machine), but I think it's good to a) preserve history (which can be relevant), and b) preserve the original commits to accredit the authors of those commits.

+1

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.

3 participants