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

[#1989] Reduce scope of try-catch block in ArgsParser::parse #2074

Merged
merged 4 commits into from
Dec 31, 2023
Merged

[#1989] Reduce scope of try-catch block in ArgsParser::parse #2074

merged 4 commits into from
Dec 31, 2023

Conversation

nseah21
Copy link
Contributor

@nseah21 nseah21 commented Dec 23, 2023

Fixes #1989

Proposed commit message

Reduce scope of try-catch block tin ArgsParser::parse

The old scope of the try block in ArgsParser::parse 
is too permissive, making it less apparent which 
methods throw the associated exceptions.

Let's reduce the scope of the try block in 
ArgsParser::parse.

Other information

NA

MarcusTXK
MarcusTXK approved these changes Dec 23, 2023
View reviewed changes
Copy link
Member

@MarcusTXK MarcusTXK left a comment

Choose a reason for hiding this comment

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

LGTM, nice job.

@MarcusTXK MarcusTXK requested a review from a team December 23, 2023 02:16
@ckcherry23
Copy link
Member

Looks good!

@chan-j-d
Copy link
Contributor

Hi @nseah21, thanks for doing up this PR! Though note that we generally restrict one first timer issue per contributor, so do look up issues without the tag for future contribution. Thanks!

@chan-j-d chan-j-d merged commit 3cd1b6e into reposense:master Dec 31, 2023
10 checks passed
@github-actions GitHub Actions
Copy link
Contributor

The following links are for previewing this pull request:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarcusTXK MarcusTXK MarcusTXK approved these changes

@ckcherry23 ckcherry23 ckcherry23 approved these changes

Assignees
No one assigned
Labels
s.ToMerge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Reduce the scope of the try-catch block in ArgsParser::parse
4 participants
@nseah21 @ckcherry23 @chan-j-d @MarcusTXK