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

rgw: cleanup unused parameters in RGWRados::copy_obj_data #18917

Merged
merged 1 commit into from Nov 17, 2017
Merged

rgw: cleanup unused parameters in RGWRados::copy_obj_data #18917

merged 1 commit into from Nov 17, 2017

Conversation

ZVampirEM77
Copy link
Contributor

@ZVampirEM77 ZVampirEM77 commented Nov 14, 2017 •
edited

src_obj, max_chunk_size, category and ptag are out of use in RGWRados::copy_obj_data.

Signed-off-by: Enming Zhang enming.zhang@umcloud.com

@ZVampirEM77
Copy link
Contributor Author

@cbodley please review this pr~ thank you~

@ZVampirEM77
Copy link
Contributor Author

Jenkins, retest this please.

joscollin
joscollin requested changes Nov 14, 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.

There are many unnecessary changes (unwanted characters and alignment changes). Please fix ?

@joscollin joscollin added the rgw label Nov 14, 2017
@ZVampirEM77
Copy link
Contributor Author

@joscollin thank you for your suggestion. But what do you mean about unwanted characters? I can not find any unwanted characters. And I think that alignment changes for aligning all parameters is OK. What do you think?

@ZVampirEM77
Copy link
Contributor Author

@cbodley @joscollin I have recommitted, please review again~ thank you~

@joscollin
Copy link
Member

joscollin commented Nov 15, 2017 •
edited

@ZVampirEM77 Sorry for the wrong term. I was referring to those alignment changes.

What I meant was: It is good to align it properly. But at a later point of time, people cannot easily recognize what (or why) you have changed in those lines when they see your name shows up in the git history (for instance git blame). In contrast, if you had made a code change in those lines, it was totally fine to align it properly. So that is the reason I gave that comment yesterday. However, it is a nit. :-)

@ZVampirEM77
Copy link
Contributor Author

@joscollin your suggestion is appreciated, thank you~ I will fix those alignment changes later.

@ZVampirEM77
rgw: cleanup unused parameters in RGWRados::copy_obj_data
3cd8511 
src_obj, max_chunk_size, category and ptag are out
of use in RGWRados::copy_obj_data.

Signed-off-by: Enming Zhang <enming.zhang@umcloud.com>
@ZVampirEM77
Copy link
Contributor Author

@joscollin I have fixed those unnecessary alignment changes, please review again~

@ZVampirEM77
Copy link
Contributor Author

Jenkins, retest this please.

@ZVampirEM77
Copy link
Contributor Author

Jenkins, retest this please.

@joscollin
Copy link
Member

@ZVampirEM77 The jenkins fix is in progress. #18964. Wait until it is finished and then do a jenkins rebuild.

@ZVampirEM77
Copy link
Contributor Author

ZVampirEM77 commented Nov 16, 2017 •
edited

@joscollin OK, thank you~

@yuriw
Copy link
Contributor

yuriw commented Nov 16, 2017

wip-yuri3-testing-2017-11-16-1724

@ZVampirEM77
Copy link
Contributor Author

Jenkins, retest this please.

1 similar comment
@ZVampirEM77
Copy link
Contributor Author

Jenkins, retest this please.

@yuriw
Copy link
Contributor

yuriw commented Nov 17, 2017

test this please

@yuriw
Copy link
Contributor

yuriw commented Nov 17, 2017

ready for merge

@yuriw yuriw merged commit f870729 into ceph:master Nov 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cbodley cbodley cbodley approved these changes

@joscollin joscollin joscollin approved these changes

Assignees
No one assigned
Labels
cleanup needs-qa rgw wip-yuri3-testing
Projects
None yet
Milestone
No milestone
4 participants
@ZVampirEM77 @joscollin @yuriw @cbodley