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.

Sign up for GitHub

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

Jump to bottom

genericapiserver: cut off more dependencies – episode 3 #40426

Merged
merged 7 commits into from
Jan 30, 2017
Merged

genericapiserver: cut off more dependencies – episode 3 #40426

merged 7 commits into from
Jan 30, 2017

Conversation

sttts
Copy link
Contributor

@sttts sttts commented Jan 25, 2017 •
edited
Loading

Compare commit subjects.

This is the first step to get apiserver_test.go and watch_test.go in pkg/genericapiserver/endpoints cutoff from k8s.io/kubernetes dependencies.

After this we have to sync client-go and then "episode 4" can go in.

approved based on #40363

@sttts sttts added area/apiserver kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. labels Jan 25, 2017
@sttts sttts added this to the 1.6 milestone Jan 25, 2017
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 25, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. release-note-label-needed labels Jan 25, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] Needs approval from an approver in each of these OWNERS Files:

We suggest the following people:
cc @thockin @csbell
You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 25, 2017
@deads2k
Copy link
Contributor

deads2k commented Jan 26, 2017 


httpstream - fine
trace.go - i think we're better off with a copy in the long run (weird dependency we'll have to cut later), but its ok
split fuzzer - definitely in favor of the change, I think we can go further, but if this solves your immediate need its a fine starting point
api/resource - you deleted all package content, just left a doc.go right?
move semantic - this global should probably be coupled to an individual scheme instead, right?

@sttts
Copy link
Contributor Author

sttts commented Jan 26, 2017 •
edited
Loading

@deads2k api/resource: I left a copy to make the cyclic appc/spec dep working. When appc is updated, we can delete that.

@sttts
Copy link
Contributor Author

sttts commented Jan 26, 2017

Will take another look at "move semantic", good point.

@deads2k
Copy link
Contributor

deads2k commented Jan 26, 2017

@deads2k api/resource: I left a copy to make the cyclic appc/spec dep working. When appc is updated, we can delete that.

Once this merges you'll make the pull upstream? After this merges, update the package to be .readonly. Also, can you remove everything but what appc is using?

@sttts
Copy link
Contributor Author

sttts commented Jan 26, 2017

Yes, will create a PR upstream to change the dependency.

@jayunit100
Copy link
Member

Thanks ! For the record we need this to import dependencies transitively from the scheduler into other projects.

@deads2k deads2k mentioned this pull request Jan 27, 2017
Merged
@sttts
Copy link
Contributor Author

sttts commented Jan 27, 2017

@jayunit100 upstream already removed the dependency by copying our Quantity implementation. So I will bump the godep.

@sttts sttts force-pushed the sttts-more-cutoffs-3 branch from dfe9fd8 to 31d0fbf Compare January 29, 2017 12:04
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 29, 2017
@sttts sttts changed the title [WIP] genericapiserver: cut off more dependencies – episode 3 genericapiserver: cut off more dependencies – episode 3 Jan 29, 2017
@sttts sttts added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Jan 29, 2017
@sttts sttts force-pushed the sttts-more-cutoffs-3 branch from 31d0fbf to aeb9cc6 Compare January 29, 2017 12:09
@sttts sttts mentioned this pull request Jan 29, 2017
Merged
1 task
k8s-github-robot pushed a commit that referenced this pull request Jan 29, 2017
Merge pull request #40605 from deads2k/generic-32-movehttpstream
4bba610 
Automatic merge from submit-queue

pkg/util: move httpstream to k8s.io/apimachinery

pick one commit from @sttts's pull #40426

This blocks some client-go splitting, so I'm picking it out and merging it separately.  It's not my commit, so its not a self-lgtm in that sense.

approved based on #40363
@sttts sttts force-pushed the sttts-more-cutoffs-3 branch 3 times, most recently from be75dd9 to c23b8ad Compare January 29, 2017 14:11
@sttts
Copy link
Contributor Author

sttts commented Jan 29, 2017

@deads2k semantic equality belongs into a scheme, I agree. Right now, it is independent from any scheme, but of course the equality funcs are fixed. Follow-up ok?

@sttts
Copy link
Contributor Author

sttts commented Jan 29, 2017

@deads2k appc/spec is bumped to the latest version, which cuts the cyclic dependency. Godep still complains if pkg/api/resource is gone. So I left yet another doc.go there.

@sttts sttts added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 29, 2017
@sttts sttts force-pushed the sttts-more-cutoffs-3 branch from e37f38e to 44ea6b3 Compare January 29, 2017 20:41
@sttts
Copy link
Contributor Author

sttts commented Jan 30, 2017

@k8s-bot test this

@sttts sttts added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 30, 2017
@sttts
Copy link
Contributor Author

sttts commented Jan 30, 2017

@deads2k ptal

@deads2k
Copy link
Contributor

deads2k commented Jan 30, 2017

lgtm

@deads2k deads2k added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 30, 2017
@deads2k
Copy link
Contributor

deads2k commented Jan 30, 2017

api.Semantic should be per scheme eventually.

@s-urbaniak
Copy link
Contributor

The bump and refactor in the rkt-kubelet LGTM, thanks! /cc @lucab @squeed

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 2cb17cc into kubernetes:master Jan 30, 2017
@sttts sttts deleted the sttts-more-cutoffs-3 branch January 30, 2017 16:33
@cgonyeo
Copy link
Contributor

cgonyeo commented Jan 30, 2017

Posthumous comment: You should probably update the appc/spec dependencies added here to point to a tagged release commit the next time one is made, as this set the dependency to a commit between releases.

k8s-github-robot pushed a commit that referenced this pull request Feb 1, 2017
Merge pull request #40658 from sttts/sttts-more-cutoffs-4
0567280 
Automatic merge from submit-queue (batch tested with PRs 40798, 40658)

genericapiserver: cut off more dependencies – episode 4

Follow-up of #40426.

TODO:
- [x] resync client-go before "genericapiserver: cutting off pkg/api deps" when #40426 went in and mirror repos are synched.

approved based on #40363
@calebamiles calebamiles modified the milestones: v1.6, 1.6 Feb 13, 2017
dims pushed a commit to dims/kubernetes that referenced this pull request Feb 8, 2018
Merge pull request kubernetes#40426 from sttts/sttts-more-cutoffs-3
c07c657 
Automatic merge from submit-queue

genericapiserver: cut off more dependencies – episode 3

Compare commit subjects.

This is the first step to get `apiserver_test.go` and `watch_test.go` in `pkg/genericapiserver/endpoints` cutoff from k8s.io/kubernetes dependencies.

After this we have to sync client-go and then "episode 4" can go in.

approved based on kubernetes#40363
akhilerm pushed a commit to akhilerm/apimachinery that referenced this pull request Sep 20, 2022
@k8s-publishing-bot
Merge pull request #40605 from deads2k/generic-32-movehttpstream
f4b7d73 
Automatic merge from submit-queue

pkg/util: move httpstream to k8s.io/apimachinery

pick one commit from @sttts's pull kubernetes/kubernetes#40426

This blocks some client-go splitting, so I'm picking it out and merging it separately.  It's not my commit, so its not a self-lgtm in that sense.

approved based on kubernetes/kubernetes#40363

Kubernetes-commit: 4bba6105657a8b7a46adf67140cb4065fff1885e
akhilerm pushed a commit to akhilerm/apimachinery that referenced this pull request Sep 20, 2022
@k8s-publishing-bot
Merge pull request #40658 from sttts/sttts-more-cutoffs-4
1cd33ef 
Automatic merge from submit-queue (batch tested with PRs 40798, 40658)

genericapiserver: cut off more dependencies – episode 4

Follow-up of kubernetes/kubernetes#40426.

TODO:
- [x] resync client-go before "genericapiserver: cutting off pkg/api deps" when #40426 went in and mirror repos are synched.

approved based on #40363

Kubernetes-commit: 056728067d762d7a7a673b2039f90529e68b78f9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@deads2k deads2k

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v1.6
Development

Successfully merging this pull request may close these issues.
9 participants
@sttts @k8s-reviewable @k8s-github-robot @deads2k @jayunit100 @s-urbaniak @cgonyeo @calebamiles @k8s-ci-robot