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

remove khmer::read_parsers::IParser::ParserState::thread_id #323

Merged
merged 1 commit into from
Mar 14, 2014
Merged

remove khmer::read_parsers::IParser::ParserState::thread_id #323

merged 1 commit into from
Mar 14, 2014

Conversation

mr-c
Copy link
Contributor

@mr-c mr-c commented Mar 12, 2014

As it is unused and uninitialized. Fixes CID 1054789

@mr-c
remove khmer::read_parsers::IParser::ParserState::thread_id
9ed132c 
As it is unused and uninitialized. CID 1054789
@ged-jenkins
Copy link

Test PASSed.
Refer to this link for build results: http://ci.ged.msu.edu/job/khmer-multi-pullrequest/267/

@mr-c
Copy link
Contributor Author

mr-c commented Mar 12, 2014

  • Is it mergable
  • Did it pass the tests?
  • [na] If it introduce new functionality in scripts/ is it tested?
    Check for code coverage.
  • Is it well formatted? Look at pep8/pylint, cppcheck, and
    make doc output. Use autopep8 and astyle -A10 if needed.
  • Is it documented in the Changelog?

@mr-c
Copy link
Contributor Author

mr-c commented Mar 12, 2014

@luizirber, @camillescott, @ctb please review and merge :-)

@mr-c mr-c added this to the 1.0 release milestone Mar 12, 2014
mr-c added a commit that referenced this pull request Mar 14, 2014
@mr-c
Merge pull request #323 from ged-lab/cid-1054789
279a73c 
remove khmer::read_parsers::IParser::ParserState::thread_id
@mr-c mr-c merged commit 279a73c into master Mar 14, 2014
@mr-c mr-c deleted the cid-1054789 branch March 14, 2014 21:04
@ctb
Copy link
Member

ctb commented Mar 16, 2014

Who did the review on this?

@mr-c
Copy link
Contributor Author

mr-c commented Mar 16, 2014

You did, outside of the elevator.

On Sun, Mar 16, 2014 at 4:31 PM, C. Titus Brown [email protected]:

Who did the review on this?

Reply to this email directly or view it on GitHubhttps://github.com//pull/323#issuecomment-37769363
.

@ctb
Copy link
Member

ctb commented Mar 16, 2014

Ahh, I feared that was the case. Could we have someone else look at the code changes, please?

mr-c
mr-c reviewed Mar 16, 2014
View reviewed changes
lib/read_parsers.hh
@@ -424,8 424,6 @@ protected:
// TODO: Set buffer size from Config.
static uint64_t const BUFFER_SIZE = 127;

uint32_t thread_id;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A guide to the reviewer: the only reference to khmer::read_parsers::IParser::ParserState::thread_id was in the (uint32_t const thread_id, uint8_t const trace_level) ParserState constructor where the supplied thread_id is recorded in the TraceLogger in lib/read_parsers.cc:1446 https://github.com/ged-lab/khmer/blob/358675f38c84289c8f7bd579b1558237a42c092f/lib/read_parsers.cc#L1446

mr-c added a commit that referenced this pull request Apr 1, 2014
@mr-c
Merge pull request #323 from ged-lab/cid-1054789
a834e66 
remove khmer::read_parsers::IParser::ParserState::thread_id
@ctb ctb mentioned this pull request Apr 20, 2014
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0 release
Development

Successfully merging this pull request may close these issues.
3 participants
@mr-c @ged-jenkins @ctb