[MISC] Remove some warnings and update parser. #164
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Irallia
commented
Sep 27, 2021
| @@ -282,7 +282,7 @@ TEST_F(iGenVar_cli_test, fail_negative_min_var_length) | |||
| "--min_var_length -30"); | |||
| std::string expected_err | |||
| { | |||
| "[Error] You gave a negative min_var_length parameter.\n" | |||
| "[Error] Value parse failed for --min_var_length: Argument -30 could not be parsed as type std::string.\n" | |||
There was a problem hiding this comment.
Why is'nt this size_t? Is this a bug of the seqan3 parser?
Suggested change
| "[Error] Value parse failed for --min_var_length: Argument -30 could not be parsed as type std::string.\n" | |
| "[Error] Value parse failed for --min_var_length: Argument -30 could not be parsed as type size_t.\n" |
There was a problem hiding this comment.
Resolved by seqan/seqan3#2836
After SeqAn3 Release 3.1. it will look like:
Suggested change
| "[Error] Value parse failed for --min_var_length: Argument -30 could not be parsed as type std::string.\n" | |
| "[Error] Value parse failed for --min_var_length: Argument -30 could not be parsed as type unsigned 64 bit integer.\n" |
Irallia
commented
Sep 27, 2021
Irallia
force-pushed
the
MISC/remove_warnings
branch
from
36019a4 to
febb5d4
Compare
Codecov Report
@@ Coverage Diff @@
## master #164 +/- ##
==========================================
+ Coverage 94.88% 96.39% +1.50%
==========================================
Files 18 18
Lines 763 749 -14
==========================================
- Hits 724 722 -2
+ Misses 39 27 -12
Continue to review full report at Codecov.
|
Signed-off-by: Lydia Buntrock <[email protected]>
Signed-off-by: Lydia Buntrock <[email protected]>
Irallia
force-pushed
the
MISC/remove_warnings
branch
from
b8478b0 to
10fb29e
Compare
Irallia
commented
Sep 27, 2021
| @@ -15,7 +15,7 @@ enum detection_methods | |||
|
|
|||
| //!\cond | |||
| // ATTENTION: Must always be the last item; will be used to determine the number of ids. | |||
| SIZE //!< Determines the size of the enum. | |||
| // SIZE //!< Determines the size of the enum. | |||
There was a problem hiding this comment.
Since we don't need this at the moment I have just commented it out.
eseiler
approved these changes
Sep 27, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #161
When you build iGenVar with
cmake ~/Repos/iGenVar/ -DCMAKE_CXX_FLAGS="-Wall -Wextra -pedantic"you will run into some warnings, which will also get fixed with this PR.I added some tests and cleaned up the CLI tests in the process. Now the file is very difficult to review. So maybe just ignore it, since my new tests all go through and are not a big innovation.