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

osd: update dmclock library w git subtree pull #17737

Merged
merged 2 commits into from Sep 17, 2017
Merged

osd: update dmclock library w git subtree pull #17737

merged 2 commits into from Sep 17, 2017

Conversation

ivancich
Copy link
Member

The dmclock library has had a number of updates, some allowing all components to be compiled with gcc 7.1.1. We should pull it in.

@ivancich
Squashed 'src/dmclock/' changes from 3408cb8f3c..a9e777f08f
5ff1277 
a9e777f08f Merge pull request ceph#35 from joscollin/wip-warning-dmclock
45c63a5b10 Merge pull request ceph#36 from ceph/wip-fix-functional
40a9fe55a6 With upgrade to gcc 7.1.1 need to include <functional> in additional header files.
a2dd155f08 src/dmclock_server.h: silence warning from -Wmaybe-uninitialized
0d6c7c6544 Merge pull request ceph#24 from bspark8/wip_online_client_info_f
a5e8cdea31 Add unit test for update_client_info, dynamic_cli_info_f.
1faf3524c6 Modify each client's QoS parameter applied time from client_rec creation time to each request's tagging creation time.

git-subtree-dir: src/dmclock
git-subtree-split: a9e777f08f288fe6834599afdeefcd9c7d31e6c1
@ivancich
Merge commit '5ff127736bc10d657ccdee9aabf179bbfa81d5c0' into wip-pull…
d72b10f 
…-updated-dmclock

Signed-off-by: J. Eric Ivancich <ivancich@redhat.com>
@ivancich ivancich added the core label Sep 14, 2017
@ivancich
Copy link
Member Author

pulpito: http://pulpito.ceph.com/ivancich-2017-09-13_22:11:33-rados-wip-pull-updated-dmclock---basic-smithi/

@joscollin
Copy link
Member

Jenkins retest this please

joscollin
joscollin requested changes Sep 15, 2017
View reviewed changes
Copy link
Member

@joscollin joscollin left a comment

Choose a reason for hiding this comment

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

One of your commits are not signed Signed-off-by:.

@ivancich
Copy link
Member Author

@joscollin Yeah, when you do a git subtree pull it turns into two commits, the first of which you don't have the opportunity to sign. Someone with sufficient rights can do the merge despite that.

@joscollin
Copy link
Member

@ivancich Do you mean you cannot amend it too ?

@joscollin
Copy link
Member

@ivancich See this attempt https://github.com/ceph/ceph/pull/17716/commits

@ivancich
Copy link
Member Author

@joscollin It's the first of two commits that git subtree pull does that needs the amending. So when you get back to the command-line it's two commits back, and to my knowledge git commit --amend cannot touch it. A git rebase -i would generally work, although there's an issue when you try to replay a commit in a git subtree -- it loses the prefix to the paths and applies the patches to the wrong files.

Can you outline the commands/process you're suggesting?

@tchaikov
Copy link
Contributor

http://pulpito.ceph.com/kchai-2017-09-15_19:26:05-rados-wip-kefu-testing-2017-09-16-0123-distro-basic-mira/

tchaikov
tchaikov approved these changes Sep 16, 2017
View reviewed changes
Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

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

i'd suggest merge as it is.

@joscollin
Copy link
Member

@ivancich I was suggesting git rebase -i sha. Sorry for the wrong word amend in the previous comment. But if it creates a problem (applies the patches to the wrong files) then leave it. May be someone with more rights can merge without Signed-off-by, not sure though.

@tchaikov
Copy link
Contributor

@ivancich for some reason, the title of this PR has [DNM] in it. so let's hold on until [DNM] is removed.

@tchaikov tchaikov removed the needs-qa label Sep 16, 2017
@joscollin
Copy link
Member

@tchaikov I think DNM is put by @ivancich himself, as the first notification email in my inbox shows @ivancich requested your review on: ceph/ceph#17737 [DNM] osd: update dmclock library w git subtree pull.

@ivancich ivancich changed the title [DNM] osd: update dmclock library w git subtree pull osd: update dmclock library w git subtree pull Sep 17, 2017
@ivancich
Copy link
Member Author

DNM removed. In the past these have been merge-able. I'm guessing @liewegas can do it.

@tchaikov tchaikov merged commit 28c8e89 into ceph:master Sep 17, 2017
@ivancich ivancich deleted the wip-pull-updated-dmclock branch October 5, 2017 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tchaikov tchaikov tchaikov approved these changes

@joscollin joscollin joscollin approved these changes

Assignees
No one assigned
Labels
core
Projects
None yet
Milestone
No milestone
3 participants
@ivancich @joscollin @tchaikov