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

UPSTREAM: cadvisor bumps #5581

Merged
merged 2 commits into from
Nov 3, 2015
Merged

Conversation

jimmidyson
Copy link
Contributor

@liggitt
Copy link
Contributor

liggitt commented Nov 2, 2015

@jimmidyson what's different? the same two upstream PRs are listed

@jimmidyson
Copy link
Contributor Author

@liggitt Something was wrong with the other PR's bumps - deleted files still existed, e.g. statvfs.c. Easier to clean up in new PR than amend other commits IMO.

@liggitt
Copy link
Contributor

liggitt commented Nov 2, 2015

k

@smarterclayton
Copy link
Contributor

Referenced from #5107

@pmorie
Copy link
Contributor

pmorie commented Nov 2, 2015

@jimmidyson One nit -- will you add the sha1 for each bump to the commit message for these? I believe that is the convention we use in origin for such things.

@deads2k Do you want these commits named differently aside from the sha1 of the bump?

@liggitt
Copy link
Contributor

liggitt commented Nov 2, 2015

should be bump(godep name): sha. no UPSTREAM needed for bumps to shared godeps

@jimmidyson
Copy link
Contributor Author

Reused @smarterclayton's commit messages... Will amend later.

@jimmidyson
Copy link
Contributor Author

Do you want each bump to each godep name in a separate commit as well? e.g. bump(github.com/google/cadvisor), bump(github.com/docker/libcontainer), etc? These were in same upstream PRs so bundled together right now.

@pweil- pweil- mentioned this pull request Nov 2, 2015
@smarterclayton
Copy link
Contributor

LGTM [merge], we can live with the poorly named commits because a rebase should update them.

@smarterclayton smarterclayton added area/performance area/reliability approved Indicates a PR has been approved by an approver from all required OWNERS files. component/metrics labels Nov 2, 2015
@smarterclayton
Copy link
Contributor

[test]

On Mon, Nov 2, 2015 at 3:41 PM, OpenShift Bot [email protected]
wrote:

continuous-integration/openshift-jenkins/merge FAILURE (
https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/3900/
)


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

@pmorie
Copy link
Contributor

pmorie commented Nov 2, 2015

[test]

@liggitt
Copy link
Contributor

liggitt commented Nov 3, 2015

github.com bower flake, re[test][merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/6673/) (Image: devenv-rhel7_2631)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 111da86

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 111da86

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/6673/)

openshift-bot pushed a commit that referenced this pull request Nov 3, 2015
@openshift-bot openshift-bot merged commit 262ea80 into openshift:master Nov 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/performance area/reliability component/metrics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants