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

Display excludeable file types and validate #178

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

danielingegneri
Copy link

@danielingegneri danielingegneri commented Oct 11, 2024

First pull req, plz let me know if needs work. #170

@kehoecj kehoecj added hacktoberfest-accepted Valid PR Hacktoberfest PR hacktoberfest 🎃 Hacktoberfest 2024 waiting-on-maintainer-review PR is waiting to be reviewed and functionally tested by the maintainers labels Oct 11, 2024
@kehoecj
Copy link
Collaborator

kehoecj commented Oct 11, 2024

@danielingegneri Thanks for the PR! Love the implementation. Looks like there is a goreportcard issue around the complexity in the *excludeFileTypesPtr - please take a look an see if you can resolve.

…nts and flag usages; moved to caller, and made the flags a var block. Unfortunately not able to get complexity below 17.
@danielingegneri
Copy link
Author

danielingegneri commented Oct 11, 2024

Attempted to reduce complexity. Mostly by simplifying validation more. Also noticed the messages being printed on validation error, and the error being returned, were repeats. Suggest instead displaying the error text in thrown error.
EDIT: Unfortunately can't get complexity below 17 without refactoring more of that function. Complexity seems warranted for the number of variables being validated.

@danielingegneri
Copy link
Author

Is it preferred that I undo the additional logging changes made, and keep to minimum?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest 🎃 Hacktoberfest 2024 hacktoberfest-accepted Valid PR Hacktoberfest PR waiting-on-maintainer-review PR is waiting to be reviewed and functionally tested by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants