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: drop unused find_replacement() and some function docs #16386

Merged
merged 1 commit into from Jul 19, 2017

Conversation

Projects
None yet
3 participants
@mikulely
Contributor

mikulely commented Jul 18, 2017

  • drop outdated & misplaced function description
  • drop unused func find_placement

Signed-off-by: Jiaying Ren jiaying.ren@umcloud.com

@mikulely mikulely changed the title from rgw: drop outdated & misplaced function description to rgw: mis trivial cleanup Jul 18, 2017

@mikulely mikulely changed the title from rgw: mis trivial cleanup to rgw: misc trivial cleanup Jul 18, 2017

@joscollin joscollin self-requested a review Jul 18, 2017

@@ -2255,7 +2255,7 @@ class RGWBucketInstanceMetadataHandler : public RGWMetadataHandler {
bci.info.bucket.tenant = tenant_name;
ret = store->select_bucket_location_by_rule(bci.info.placement_rule, &rule_info);
if (ret < 0) {
ldout(store->ctx(), 0) << "ERROR: select_bucket_placement() returned " << ret << dendl;
ldout(store->ctx(), 0) << "ERROR: select_bucket_location_by_rule() returned " << ret << dendl;

This comment has been minimized.

@joscollin

joscollin Jul 18, 2017

Member

Can you use __func__ here ?

This comment has been minimized.

@mikulely

mikulely Jul 18, 2017

Contributor

OK, I think we'd better drop this, because we want to rename this (like https://github.com/ceph/ceph/pull/15797/files#diff-c30965955342b98393b73be699f4e355R5890 ) finally.

@mikulely

This comment has been minimized.

Contributor

mikulely commented Jul 18, 2017

@joscollin repushed, plz review.

@joscollin

This comment has been minimized.

@mikulely

This comment has been minimized.

Contributor

mikulely commented Jul 18, 2017

@joscollin sqaushed .: -)

/**
* create a bucket with name bucket and the given list of attrs
* returns 0 on success, -ERR# otherwise.
*/
int init_bucket_index(RGWBucketInfo& bucket_info, int num_shards);

This comment has been minimized.

@joscollin

joscollin Jul 18, 2017

Member

I think it would be better to move this comment to create_bucket() declaration in the same file, instead of dropping it.

This comment has been minimized.

@cbodley

cbodley Jul 18, 2017

Contributor

it's amusing to see how far that comment has traveled from its original home :)

This comment has been minimized.

@joscollin

joscollin Jul 18, 2017

Member

@cbodley Yeah! At first observation, I thought of merging it. But why to drop, if the function still exist in the same file :-)

This comment has been minimized.

@mikulely

mikulely Jul 19, 2017

Contributor

@joscollin @cbodley The comment was introduced by 62e1111

-
+/**
+ * create a bucket with name bucket and the given list of attrs
+ * returns 0 on success, -ERR# otherwise.
+ */
 int RGWRados::create_bucket(std::string& id, std::string& bucket, map<nstring, bufferlist>& attrs)
 {

For current method prototype , the comment is outdated. I'd prefer to drop this. What do you think?

This comment has been minimized.

@joscollin

joscollin Jul 19, 2017

Member

@mikulely Yeah, you are right. If you think dropping it then drop the same in the implementation too rgw_rados.cc; or update it properly as per the new prototype. If I was doing it, I would prefer updating it as per the latest prototype.

This comment has been minimized.

@mikulely

mikulely Jul 19, 2017

Contributor

Thanks @joscollin for reminding, current prototype is tedious and obvious, can we just drop the outdated doc?

rgw: miis trivial cleanup
+ drop outdated & misplaced function description
+ drop unused func find_placement

Signed-off-by: Jiaying Ren <jiaying.ren@umcloud.com>

@joscollin joscollin changed the title from rgw: misc trivial cleanup to rgw: drop find_replacement() and some function docs Jul 19, 2017

@joscollin joscollin changed the title from rgw: drop find_replacement() and some function docs to rgw: drop unused find_replacement() and some function docs Jul 19, 2017

@joscollin joscollin merged commit ff05c2e into ceph:master Jul 19, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details

@mikulely mikulely deleted the mikulely:rgw-cleanup branch Jul 25, 2017

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