Fix/508 for code review

[iglpdc]

I have a first version of the code. I've made a new base class for khmer exceptions that derives from std::exception.

A couple of things, though:

• I think the exceptions defined in khmer.hh should be moved to the new file khmer_exceptions.hh.
• when grepping the cc files for std::exception, a few functions throw std::exception's instead of khmer derived exceptions. I think this should be changed in most cases.

[ged-jenkins]

Can one of the admins verify this patch?

[ged-jenkins]

Can one of the admins verify this patch?

[mr-c]

ok to test

[mr-c]

@iglpdc This looks great. I agree about the other std::exceptions, they need to be changed

[mr-c]

I'm also +1 for consolidating all the exceptions into this new file

[ctb]

I'm OK with EITHER merging this first, OR waiting for @iglpdc to do more work :). Which would you prefer, Ivan?

[iglpdc]

I can move all the exceptions to this new file. I can try to change all the throw std::exception()in the code to the new khmer::exception. We can merge after that.

[ctb]

On Wed, Jul 23, 2014 at 07:49:21AM -0700, Ivan Gonzalez wrote:

I can move all the exceptions to this new file. I can try to change all the throw std::exception()in the code to the new khmer::exception. We can merge after that.

👍

[iglpdc]

I've moved all the exceptions defined in khmer.hh to the new file khmer_exceptions.hhand made changes so the code throws only khmer::khmer_exception or its derived classes. Probably, in some cases it's still too generic, but I don't know the code enough to introduce new exceptions :-).

In any case, take a look.

[ctb]

LGTM! Can you paste in the checklist and check off as many of the boxes as possible?

[iglpdc]

• Is it mergable
• Did it pass the tests?
• If it introduces 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 --max-code-length=80
  if needed.
• Is it documented in the Changelog?
• Was spellcheck run on the source code and documentation after changes
  were made

[mr-c]

It can be tedious, but we list every file modified in our changelog entries (see below)

[mr-c]

Includes should all be at the top together

[mr-c]

@iglpdc This is great, thanks! Fix the comments and I'll merge

[iglpdc]

Just moved the include's altogether.

[mr-c]

@iglpdc Congratulations on your first commit to the khmer project! Your name will be included in the release notes for the next version and you'll be listed amongst our other contributors in the next software release paper.

[ged-jenkins]

Can one of the admins verify this patch?

[ged-jenkins]

Can one of the admins verify this patch?

[iglpdc]

Awesome!

Ivan

El 23/07/2014, a las 23:00, "Michael R. Crusoe" notifications@github.com escribió:

@iglpdc Congratulations on your first commit to the khmer project! Your name will be included in the release notes for the next version and you'll be listed amongst our other contributors in the next software release paper.

—
Reply to this email directly or view it on GitHub.