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

Makes the exception names of the argument parser consistent #1467

Merged
merged 2 commits into from
Feb 4, 2020

Conversation

smehringer
Copy link
Member

@smehringer smehringer commented Jan 7, 2020

Resolves #481

I changed the names according to our resolution in the strategy meeting (card)
Additionally, the first commit get's rid of the internal exception in the function retrive_value. The success is indicated by a return value instead and the exception is thrown outside. This also made the exceptions std::type_conversion-error and std::overflow_errow obsolete which lowers the number of exceptions in general.

@smehringer smehringer force-pushed the argument_parser_misc branch from fe5b951 to 70bb020 Compare January 7, 2020 13:01
@smehringer smehringer changed the title Argument parser misc Makes the exception names of the argument parser consistent Jan 7, 2020
@smehringer smehringer force-pushed the argument_parser_misc branch from 70bb020 to bf64382 Compare January 8, 2020 09:15
@codecov
Copy link

codecov bot commented Jan 8, 2020

Codecov Report

Merging #1467 into master will increase coverage by 0.04%.
The diff coverage is 86.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1467      +/-   ##
==========================================
+ Coverage   97.58%   97.62%   +0.04%     
==========================================
  Files         235      235              
  Lines        8844     8855      +11     
==========================================
+ Hits         8630     8645      +15     
+ Misses        214      210       -4
Impacted Files Coverage Δ
include/seqan3/argument_parser/exceptions.hpp 100% <100%> (ø) ⬆️
include/seqan3/argument_parser/validators.hpp 88.69% <44.44%> (ø) ⬆️
include/seqan3/argument_parser/argument_parser.hpp 98.54% <93.75%> (ø) ⬆️
...ude/seqan3/argument_parser/detail/format_parse.hpp 96.58% <97.5%> (+2.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92dfc95...a5ca7d4. Read the comment docs.

@smehringer smehringer requested a review from MitraDarja January 8, 2020 11:32
Copy link
Contributor

@MitraDarja MitraDarja left a comment

Choose a reason for hiding this comment

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

Just some typos. :)

CHANGELOG.md Outdated Show resolved Hide resolved
include/seqan3/argument_parser/argument_parser.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/detail/format_parse.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/detail/format_parse.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/detail/format_parse.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/detail/format_parse.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/detail/format_parse.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/exceptions.hpp Outdated Show resolved Hide resolved
@smehringer smehringer requested a review from MitraDarja January 8, 2020 12:32
@smehringer
Copy link
Member Author

sorry I did not see that you approved already. You can or cannot check the differences and approve again. In general if you find typos, you can request changes as they are just as valid as code changing requests.

@smehringer smehringer requested a review from rrahn January 8, 2020 12:37
@smehringer smehringer force-pushed the argument_parser_misc branch from ec5002a to 9a15ab1 Compare January 9, 2020 11:38
include/seqan3/argument_parser/argument_parser.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/detail/format_parse.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/detail/format_parse.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/detail/format_parse.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/detail/format_parse.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/validators.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/exceptions.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/exceptions.hpp Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@smehringer smehringer force-pushed the argument_parser_misc branch from 9a15ab1 to 9cc42bc Compare January 10, 2020 12:54
@smehringer
Copy link
Member Author

Sorry, I had to rebase and add a different commit for the renaming in the documentation to keep the history clean. Doing that later would have been too compilcated.

@smehringer smehringer force-pushed the argument_parser_misc branch from 9cc42bc to 8f3b0ca Compare January 10, 2020 13:10
@smehringer smehringer requested a review from rrahn January 10, 2020 13:12
@smehringer smehringer force-pushed the argument_parser_misc branch from 8f3b0ca to 9cbd5d8 Compare January 10, 2020 21:32
Copy link
Contributor

@rrahn rrahn left a comment

Choose a reason for hiding this comment

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

a few things left

include/seqan3/argument_parser/argument_parser.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/detail/format_parse.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/detail/format_parse.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/detail/format_parse.hpp Outdated Show resolved Hide resolved
include/seqan3/argument_parser/detail/format_parse.hpp Outdated Show resolved Hide resolved
@smehringer smehringer force-pushed the argument_parser_misc branch from 85af7ba to b89e594 Compare January 14, 2020 12:09
@smehringer smehringer requested a review from rrahn January 14, 2020 12:09
@smehringer smehringer force-pushed the argument_parser_misc branch from b89e594 to d029b64 Compare January 15, 2020 10:24
@rrahn
Copy link
Contributor

rrahn commented Jan 16, 2020

Have you verified the code coverage issue?

@smehringer
Copy link
Member Author

It has nothing to do with me. I'll rebase to current master to see if this fixes it.

@smehringer smehringer force-pushed the argument_parser_misc branch from d029b64 to 34f548f Compare January 16, 2020 12:32
@smehringer
Copy link
Member Author

@rrahn Code coverage only fails because of the filesystem errors that in the file validators were renamed but are not tested. So I think this can be merged.

@smehringer smehringer force-pushed the argument_parser_misc branch from 34f548f to a5ca7d4 Compare February 3, 2020 15:13
@smehringer
Copy link
Member Author

@rrahn I updated the Changelog entry to link this PR.
The code coverage fail is unrelated to this PR, since it records 4 misses that are due to the untested error handling of the input_directory_validators where I changed the exception names so I trigger the misses again. Fixes the code coverage should be part of another PR, I'll make a card.

@smehringer smehringer merged commit 5b88fc6 into seqan:master Feb 4, 2020
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.

[Argument Parser] make the exception names consistent
3 participants