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

Check if lookupDir exists before iterating #1527

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

syyyr
Copy link
Contributor

@syyyr syyyr commented Nov 15, 2023

std::filesystem::recursive_directory_iterator throws if the directory doesn't exist.

@Framstag
Copy link
Owner

@syyyr : Can you fix the code in regard to @Karry comments?

std::filesystem::recursive_directory_iterator throws if the directory
doesn't exist.
@syyyr
Copy link
Contributor Author

syyyr commented Nov 20, 2023

By the way, are you okay with me just sending fixes like these? It seemed to me like a quick and easy fix and for those, I tend to make PRs straight away without first making an issue with an exact explanation of the problem (and a reproducer). I wouldn't want to spam the repo with fixes that can't be reproduced by anyone else, so feel free to tell me if I should change my approach in any way. Thanks.

Copy link

sonarcloud bot commented Nov 20, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Framstag
Copy link
Owner

By the way, are you okay with me just sending fixes like these? It seemed to me like a quick and easy fix and for those, I tend to make PRs straight away without first making an issue with an exact explanation of the problem (and a reproducer). I wouldn't want to spam the repo with fixes that can't be reproduced by anyone else, so feel free to tell me if I should change my approach in any way. Thanks.

Libosmscout is a library for its users. As such I definitely accept such patches, if they improve the overall codebase and its stability or resilliance. Or add features that make sense in the code base.
I'm more. Strict if it comes to portability, dependencies or similar high impact stuff

Copy link
Collaborator

@Karry Karry left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@Karry
Copy link
Collaborator

Karry commented Nov 21, 2023

By the way, are you okay with me just sending fixes like these? It seemed to me like a quick and easy fix and for those, I tend to make PRs straight away without first making an issue with an exact explanation of the problem (and a reproducer). I wouldn't want to spam the repo with fixes that can't be reproduced by anyone else, so feel free to tell me if I should change my approach in any way. Thanks.

I am more than happy with such merge requests ;-) I am creating issues just in case when I have no time for fix it right away. When it is simple like this one, there is no reason to waste time with describing problem in issue. Just my opinion.

@Framstag Framstag merged commit e43fa19 into Framstag:master Nov 22, 2023
18 of 20 checks passed
Copy link

github-actions bot commented Jun 2, 2024

🎉 This issue has been resolved in v2024.06.02.1 (Release Notes)

@github-actions github-actions bot added the released Issue has been released label Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Issue has been released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants