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

fix: report directory is not exist #207

Merged
merged 2 commits into from
Feb 20, 2024

Conversation

nargov
Copy link
Contributor

@nargov nargov commented Feb 17, 2024

Closes #200

I would usually start a discussion about a change before starting, but since this was just a tiny fix I skipped it this time. Hope that's Ok.

This adds a path existence check to the path(s) passed in and ensures the directory exists.

I submit this contribution under the Apache-2.0 license.

reporting/report.go Fixed Show fixed Hide fixed
reporting/report.go Outdated Show resolved Hide resolved
reporting/report.go Outdated Show resolved Hide resolved
Copy link
Contributor

@baruchiro baruchiro left a comment

Choose a reason for hiding this comment

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

Welcome!
Thank you for your contribution, indeed, no extra communication is needed here before starting :-)

reporting/report.go Fixed Show resolved Hide resolved
Copy link
Contributor

@baruchiro baruchiro left a comment

Choose a reason for hiding this comment

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

One more iteration 😉

reporting/report_test.go Show resolved Hide resolved
@baruchiro baruchiro changed the title fix #200 - check report path exists, otherwise create it fix: report directory is not exist (#200) Feb 18, 2024
@baruchiro baruchiro changed the title fix: report directory is not exist (#200) fix: report directory is not exist Feb 18, 2024
reporting/report.go Outdated Show resolved Hide resolved
auto-merge was automatically disabled February 18, 2024 18:28

Head branch was pushed to by a user without write access

@nargov
Copy link
Contributor Author

nargov commented Feb 18, 2024

Funny, it failed on permission for writing to the temp folder.
I'll revert that change.

auto-merge was automatically disabled February 18, 2024 20:10

Head branch was pushed to by a user without write access

@baruchiro
Copy link
Contributor

baruchiro commented Feb 19, 2024

I tested it on #209 and if you set the permission of MkdirAll to be 0750 it will work.

I think 0750 is OK, WDYT?

@baruchiro baruchiro added this pull request to the merge queue Feb 20, 2024
Merged via the queue into Checkmarx:master with commit 9fe6a3d Feb 20, 2024
8 checks passed
@baruchiro
Copy link
Contributor

Well done and huge thanks!

BTW, why do you force push every time?

@nargov
Copy link
Contributor Author

nargov commented Feb 20, 2024

To keep the history to a single commit (I use commit --amend, and it requires rewriting the branch history). :)

I'll try something else soon.

@nargov nargov deleted the report-path-creation branch February 20, 2024 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"no such file or directory" for report-path
2 participants