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

rgw: remove useless lines in RGWDeleteBucket::execute #19699

Merged
merged 1 commit into from Jan 11, 2018

Conversation

qrGitHub
Copy link

No description provided.

@@ -2896,7 +2896,6 @@ void RGWDeleteBucket::execute()
}

string prefix, delimiter;

Copy link
Contributor

Choose a reason for hiding this comment

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

Plz do not introduce this kind of cleanup, if you're not touching around. This will pollute the commit history, which make us can not use git show and git blame to dig commit history.

Copy link
Contributor

Choose a reason for hiding this comment

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

This also introduces the headache for resolving rebase conflict.

Copy link
Author

Choose a reason for hiding this comment

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

Do you mean don't delete blank lines?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes.

@@ -2911,13 +2910,11 @@ void RGWDeleteBucket::execute()
}

op_ret = abort_bucket_multiparts(store, s->cct, s->bucket_info, prefix, delimiter);

Copy link
Contributor

Choose a reason for hiding this comment

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

same

if (op_ret < 0) {
return;
}

op_ret = store->delete_bucket(s->bucket_info, ot, false);

Copy link
Contributor

Choose a reason for hiding this comment

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

same

@@ -2872,7 +2872,7 @@ void RGWDeleteBucket::execute()
}

op_ret = rgw_bucket_sync_user_stats(store, s->user->user_id, s->bucket_info);
if ( op_ret < 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Copy link
Author

Choose a reason for hiding this comment

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

@mikulely What about this one? Someone once let me delete the extra spaces.
Are there any guidelines for spaces and blank lines?

Copy link
Member

Choose a reason for hiding this comment

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

sometimes whitespace changes do not get picked up while reviewing, otherwise introducing purely whitesepace changes makes it difficult when backporting, or when git blame etc.

Signed-off-by: Bingyin Zhang <zhangbingyin@cloudin.cn>
@qrGitHub
Copy link
Author

@mikulely @ZVampirEM77 @theanalyst, remove unnecessary changes according to your advices, thanks.

@qrGitHub
Copy link
Author

qrGitHub commented Jan 8, 2018

Hi @cbodley @adamemerson, would you help to review this? Thanks.

@cbodley
Copy link
Contributor

cbodley commented Jan 8, 2018

jenkins test docs please

@qrGitHub
Copy link
Author

Hi @cbodley, can this be merged? Thanks.

@theanalyst
Copy link
Member

jenkins test docs

@cbodley cbodley merged commit 29816d2 into ceph:master Jan 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants