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

ConfigMap Volume #20114

Merged
merged 2 commits into from Feb 15, 2016
Merged

ConfigMap Volume #20114

merged 2 commits into from Feb 15, 2016

Conversation

pmorie
Copy link
Member

@pmorie pmorie commented Jan 25, 2016

Do you like volumes? Do you like ConfigMaps? If so, this is the PR for you.

@k8s-github-robot
Copy link

Labelling this PR as size/XS

@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/new-api size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 25, 2016
@k8s-bot
Copy link

k8s-bot commented Jan 26, 2016

GCE e2e test build/test passed for commit a797284c37d5793d2c145a69b5bdf6ec1036effb.

@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 26, 2016
@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 26, 2016
@k8s-bot
Copy link

k8s-bot commented Jan 26, 2016

GCE e2e test build/test passed for commit 80ac31520b5e558be097ede1721768cd3ed464a0.

@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 27, 2016
@pmorie pmorie mentioned this pull request Jan 28, 2016
Closed
12 tasks
@bgrant0607 bgrant0607 assigned thockin and unassigned bgrant0607 Jan 28, 2016
@bgrant0607
Copy link
Member

Is the move to api/v1 only in this PR, or is there another PR for that?

@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, 2016
@k8s-github-robot
Copy link

Labelling this PR as size/XXL

@k8s-github-robot k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed kind/new-api size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 29, 2016
@k8s-bot
Copy link

k8s-bot commented Jan 29, 2016

GCE e2e build/test failed for commit 2b67ef38998f2201d80502135032cee90c213872.

@k8s-bot
Copy link

k8s-bot commented Jan 29, 2016

GCE e2e build/test failed for commit 5740cccc54ddc9a2f07946213855418b8224ef45.

@pmorie
Copy link
Member Author

pmorie commented Jan 31, 2016

@bgrant0607 API move was in #19716, this was implemented on top of that one.

@pmorie
Copy link
Member Author

pmorie commented Jan 31, 2016

@thockin to limit the size of this PR, I'm going to open another PR for extracting the atomic write logic, then rebase this onto that one to use it. Alternately, we could review and merge this one as is and I will add the updating feature in a subsequent PR

@k8s-github-robot
Copy link

PR needs rebase

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 2, 2016
@thockin thockin added this to the v1.2 milestone Feb 3, 2016
@pmorie
Copy link
Member Author

pmorie commented Feb 14, 2016

@thockin @freehan rebased, but I decline to squash because the second commit is generated stuff which is a pain to rebase, not to mention more layerlike than many of the commits in multi-commit merges in the last hundred commits or so...

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 14, 2016
@ghost
Copy link

ghost commented Feb 14, 2016

The problem with not squashing is that there is a point in the commit
history that won't compile, which makes things like git bisect sort of
awful. I have a few minutes now, will take a look.
On Feb 14, 2016 11:07 AM, "Paul Morie" notifications@github.com wrote:

@thockin https://github.com/thockin @freehan
https://github.com/freehan rebased, but I decline to squash because the
second commit is generated stuff which is a pain to rebase, not to mention
more layerlike than many of the commits in multi-commit merges in the last
hundred commits or so...

—
Reply to this email directly or view it on GitHub
#20114 (comment)
.

@pmorie
Copy link
Member Author

pmorie commented Feb 14, 2016

@thockin-cc I thought the guidance on squashing specifically says that it's not necessary to ensure that the build works after each commit -- I do see your point, but have never squashed regeneration before for the reasons I mentioned earlier. If it is really critical to maintaining that invariant, we should invest in having some tooling that makes it easier to rebase all this generated code.

@k8s-bot
Copy link

k8s-bot commented Feb 14, 2016

GCE e2e test build/test passed for commit 4e002972c41d2dbb944787608219ed198526c178.

@pmorie
Copy link
Member Author

pmorie commented Feb 14, 2016

@thockin what about making the merge-bot able to squash certain types of commits before final merge? Hrm...

@thockin thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 14, 2016
@thockin thockin assigned thockin and unassigned freehan Feb 14, 2016
@thockin
Copy link
Member

thockin commented Feb 14, 2016

It's fine. We are not exactly consistent on it.

@pmorie
Copy link
Member Author

pmorie commented Feb 14, 2016

@thockin I think I can get away with it now, if you have a burning desire. LMK.

Your humble servant,

P

@k8s-github-robot
Copy link

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

@ghost
Copy link

ghost commented Feb 14, 2016

Don't touch, let the mergebot do its thing
On Feb 14, 2016 11:40 AM, "k8s-merge-robot" notifications@github.com
wrote:

@k8s-bot https://github.com/k8s-bot test this [submit-queue is
verifying that this PR is safe to merge]

—
Reply to this email directly or view it on GitHub
#20114 (comment)
.

@pmorie
Copy link
Member Author

pmorie commented Feb 14, 2016

👉 👉

@k8s-bot
Copy link

k8s-bot commented Feb 14, 2016

GCE e2e test build/test passed for commit 4e002972c41d2dbb944787608219ed198526c178.

@k8s-github-robot
Copy link

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

@k8s-bot
Copy link

k8s-bot commented Feb 14, 2016

GCE e2e build/test failed for commit 4e002972c41d2dbb944787608219ed198526c178.

@k8s-github-robot
Copy link

PR changed after LGTM, removing LGTM.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 14, 2016
@pmorie pmorie added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 14, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 14, 2016

GCE e2e test build/test passed for commit 8323bb1.

@k8s-github-robot
Copy link

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

@k8s-bot
Copy link

k8s-bot commented Feb 15, 2016

GCE e2e test build/test passed for commit 8323bb1.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

k8s-github-robot pushed a commit that referenced this pull request Feb 15, 2016
Merge pull request #20114 from pmorie/config-volume
78082e2 
Auto commit by PR queue bot
@k8s-github-robot k8s-github-robot merged commit 78082e2 into kubernetes:master Feb 15, 2016
@erictune
Copy link
Member

The real problem is not so much squashing or not. It is checking in
generated files.
On Feb 14, 2016 4:16 PM, "k8s-merge-robot" notifications@github.com wrote:

Automatic merge from submit-queue

—
Reply to this email directly or view it on GitHub
#20114 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@thockin thockin

Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

None yet