-
Notifications
You must be signed in to change notification settings - Fork 196
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
add module support #215
add module support #215
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: liggitt The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold |
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.
I don't get the "add multiple packages at once" commit - it looks like a code cleanup but semantically the same - am I missing something?
Overall I get what you're doing here. I hope we can rip out a lot of this and just use tools/packages, and drop all the logic we do to recreate things like "pkg/..." support.
You know what this need - test cases to make sure we don't backslide |
@liggitt: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@liggitt any chance you are still interested in pursuing this, or, if not, may I? |
I have a k/k branch WIP which makes the codegens work in module mode AND
the new "workspace" mode (go 1.18) with `packges.Load`. The changes are
"significant" and not ready for integration. My goal is to make workspaces
work, then I will circle back and clean up the gengo hacks (and they are
indeed hacks). To hack, I forked it to a local "gengo2" directory.
https://github.com/thockin/kubernetes/tree/go-workspaces/gengo2
Note that the way the Makefile is written tries to run each tool one time
for ALL modules. This *only* works in workspace mode (which requires go
1.18). To support modules without workspaces, we would need to invoke each
tool once for each module -- muuuuuuuch slower.
Comments are welcome, but please note the goal was NOT "a clean set of
patches" yet.
in particular:
thockin/kubernetes@05c1125
thockin/kubernetes@61bef5c
thockin/kubernetes@6c1fc95
If anything, I hope to land this during the 1.25 cycle, after go 1.18 is
mandated.
…On Thu, Jan 13, 2022 at 7:41 AM Jordan Liggitt ***@***.***> wrote:
Closed #215 <#215>.
—
Reply to this email directly, view it on GitHub
<#215 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABKWAVB42CFX2VBQ3DQ4TWLUV3XCBANCNFSM5E3RLMAA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
<kubernetes/gengo/pull/215/issue_event/5888507638 <(588)%20850-7638>@
github.com>
|
Hoisted out of kubernetes/kubernetes#99226
xref kubernetes/kubernetes#82531
Need to ensure this works in k8s.io/kubernetes in GOPATH mode (kubernetes/kubernetes#105299) and module mode (kubernetes/kubernetes#99226) before merge
/cc @sttts
/sig api-machinery