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

commitAuthors:bugfix - fix when pass invalid line to SetCommitAuthors #978

Merged
merged 1 commit into from
Feb 10, 2022

Conversation

iancardosozup
Copy link
Contributor

@iancardosozup iancardosozup commented Feb 9, 2022

Fix a bug in internal/utils/file.go where line value wasn't being restored to 0 when loop restarted, adding error return to GetDependencyCodeFilepathAndLine and GetDependencyInfo and some logging messages when this error is found. Removed unused function ReplacePathSeparator. Also changed some LogError to LogErrorWithLevel and setting errors to analysisError to be print on analysis end

Signed-off-by: Ian Cardoso [email protected]

- What I did

- How to verify it

- Description for the changelog

@wiliansilvazup wiliansilvazup added kind/bug Something isn't working kind/improvement This issue is not a Bug nor a Feature labels Feb 10, 2022
@iancardosozup iancardosozup force-pushed the bugfix/commit-authors branch 2 times, most recently from e147108 to 6e0972a Compare February 10, 2022 17:37
… and changed formatters error handling approach

Fix a bug in internal/utils/file.go where line value wasn't being restored to 0 when loop restarted, instead of ignoring erros and adding vulnerabilities to analysis slices now we ignore vulnerabilities we can't parse and add an error log to final analysis.
 Also added error return to GetDependencyCodeFilepathAndLine and GetDependencyInfo and some logging messages when this error is found. Removed unused function ReplacePathSeparator. Also changed some LogError to LogErrorWithLevel and setting errors to analysisError to be print on analysis end.

Signed-off-by: Ian Cardoso <[email protected]>
@nathanmartinszup
Copy link
Contributor

I will approve this pull request, but I believe that in the near future it will be necessary to improve this formatter structure.There is a lot of duplicated code, I believe we could have something similar to the default formatter of the engine, leaving only the parse of the output for each tool, I also don't see much sense in the separation between the engines and formatters package, just complicating the project structure.

@matheusalcantarazup
Copy link
Contributor

I will approve this pull request, but I believe that in the near future it will be necessary to improve this formatter structure.There is a lot of duplicated code, I believe we could have something similar to the default formatter of the engine, leaving only the parse of the output for each tool, I also don't see much sense in the separation between the engines and formatters package, just complicating the project structure.

This is definitely true. I've already thought about some possible improvements to this, I think we should discuss it.

@nathanmartinszup nathanmartinszup merged commit 0c1e691 into main Feb 10, 2022
nathanmartinszup pushed a commit that referenced this pull request Feb 10, 2022
… and changed formatters error handling approach (#978)

Fix a bug in internal/utils/file.go where line value wasn't being restored to 0 when loop restarted, instead of ignoring erros and adding vulnerabilities to analysis slices now we ignore vulnerabilities we can't parse and add an error log to final analysis.
 Also added error return to GetDependencyCodeFilepathAndLine and GetDependencyInfo and some logging messages when this error is found. Removed unused function ReplacePathSeparator. Also changed some LogError to LogErrorWithLevel and setting errors to analysisError to be print on analysis end.

Signed-off-by: Ian Cardoso <[email protected]>
(cherry picked from commit 0c1e691)
@nathanmartinszup nathanmartinszup deleted the bugfix/commit-authors branch March 8, 2022 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working kind/improvement This issue is not a Bug nor a Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants