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

test/old: Removed commented code - 1 #15366

Merged
merged 1 commit into from May 30, 2017

Conversation

Projects
None yet
3 participants
@joscollin
Member

joscollin commented May 30, 2017

There were lot of commented code inside src/test/old/testcrush.cc. This was found while doing
the PR#15364. So Removed them.

Signed-off-by: Jos Collin jcollin@redhat.com

test/old: Removed commented code
There were lot of commented code inside src/test/old/testcrush.cc. This was found while doing
the PR#15364. Removed them.

Signed-off-by: Jos Collin <jcollin@redhat.com>

@liewegas liewegas added the needs-qa label May 30, 2017

@liewegas liewegas merged commit 57bef92 into ceph:master May 30, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@joscollin joscollin deleted the joscollin:wip-cleanup-test-remove-commented-code branch May 31, 2017

@joscollin joscollin changed the title from test/old: Removed commented code to test/old: Removed commented code - 1 Jun 7, 2017

@amitkumar50

Irrelevant code, blank lines removed.
Changes look good.

@@ -161,8 +138,6 @@ int main()
root = make_hierarchy(c, wid, ndisks, nbuckets);
}
// rule

This comment has been minimized.

@amitkumar50

amitkumar50 Jul 18, 2017

Contributor

I believe this can also be removed...

@joscollin

This comment has been minimized.

Member

joscollin commented Jul 18, 2017

@amitkumar50 Those blank lines are unnecessary. Only one blank line is expected. And this is just a cleanup PR -> No need to create another PR just for removing those blank lines.

@amitkumar50

As per my comments, Kindly address my below comments. Rest all is good.

@@ -44,7 +28,6 @@ Bucket *make_bucket(Crush& c, vector<int>& wid, int h, int& ndisks, int& nbucket
UniformBucket *b = new UniformBucket(nbuckets--, 1, 0, 10, disks);
b->make_primes(hash);
c.add_bucket(b);
//cout << h << " uniformbucket with " << wid[h] << " disks" << endl;
return b;
} else {
// mixed

This comment has been minimized.

@amitkumar50

amitkumar50 Jul 18, 2017

Contributor

mixed can also be removed

This comment has been minimized.

@joscollin

joscollin Jul 18, 2017

Member

@amitkumar50

That is an informative comment in the if {} else {} that indicates UniformBucket or MixedBucket is to be made. It improves the readability of the code too, when someone reads it for the first time.

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