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

Removed an extrenuous using namespace khmer; in kmer.hh, #276

Closed
wants to merge 1 commit into from

Conversation

fishjord
Copy link
Contributor

fixed resulting errors

@ged-jenkins
Copy link

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

@mr-c
Copy link
Contributor

mr-c commented Jan 28, 2014

Great, thanks for the eagle eye. Since I'm trying to get release out right I'm going to hold back on merging this.

I agree that none of the headers should be polluting the namespace. However I have no desire to constantly prefixing our already verbose datatypes with khmer::. We should add a 'using namespace khmer" to the top of all .cc files instead of prefixing.

@fishjord
Copy link
Contributor Author

So long as it's consistent, half the types were prefixed in
_khmermodule.cc and half weren't. I'm looking at you @ctb,
@camillescott!

Although if we're just going to toss using namepsaces at the top of
files one might wonder why we use namespaces in the first place...

On Tue, Jan 28, 2014 at 6:14 PM, Michael R. Crusoe
notifications@github.com wrote:

Great, thanks for the eagle eye. Since I'm trying to get release out right
I'm going to hold back on merging this.

I agree that none of the headers should be polluting the namespace. However
I have no desire to constantly prefixing our already verbose datatypes with
khmer::. We should add a 'using namespace khmer" to the top of all .cc files
instead of prefixing.

Reply to this email directly or view it on GitHub.

@mr-c
Copy link
Contributor

mr-c commented Feb 3, 2014

I've taken an alternative approach in #285

Comments are welcome there. I'm going to close this PR for now.

@mr-c mr-c closed this Feb 3, 2014
@mr-c mr-c deleted the pr_remove_using_kmer_hh branch March 15, 2014 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants