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

[#1881] Check if author config file exists instead of detecting it via an exception #2007

Merged
merged 1 commit into from
Jul 10, 2023
Merged

[#1881] Check if author config file exists instead of detecting it via an exception #2007

merged 1 commit into from
Jul 10, 2023

Conversation

germainelee02
Copy link
Contributor

@germainelee02 germainelee02 commented Jun 18, 2023 •
edited by chan-j-d
Loading

Fixes #1881

Currently, for both detection of author config and group config, it relies 
on running the code normally as if the file is always present and 
catching the FileNotFoundException to determine 
if the file is not present.

Let's refactor the code to detect if the file does not exist using 
an `if` block instead of catching it in the try-catch 
block

sikai00
sikai00 requested changes Jun 18, 2023
View reviewed changes
Copy link
Member

@sikai00 sikai00 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for doing this up!

  1. We include the issue that we are fixing in the title so that it is easier to track from the pull requests list. So something like [#1881] Check if... in your case.
  2. For the commit message, keep to 80 characters per line.

Other than that LGTM.

@germainelee02 germainelee02 changed the title Check if author config file exists instead of detecting it via an exc… [#1881] Check if author config file exists instead of detecting it via an exception Jun 19, 2023
@ckcherry23 ckcherry23 requested a review from a team June 20, 2023 18:32
sikai00
sikai00 approved these changes Jun 20, 2023
View reviewed changes
Copy link
Member

@sikai00 sikai00 left a comment

Choose a reason for hiding this comment

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

LGTM

@sikai00 sikai00 requested a review from a team June 21, 2023 08:23
chan-j-d
chan-j-d approved these changes Jun 21, 2023
View reviewed changes
Copy link
Contributor

@chan-j-d chan-j-d left a comment

Choose a reason for hiding this comment

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

LGTM! Good work on your first PR!

@chan-j-d chan-j-d merged commit c84383f into reposense:master Jul 10, 2023
9 of 10 checks passed
@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

@sikai00 sikai00 sikai00 approved these changes

@chan-j-d chan-j-d chan-j-d 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.

Check if author config file exists instead of detecting it via an exception
3 participants
@germainelee02 @sikai00 @chan-j-d