-
-
Notifications
You must be signed in to change notification settings - Fork 308
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
r.watershed: fix streams and basins #3140
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch!
Underlines the advantages with always-use-braces-policy!
(Could probably have been prevented by ClangFormat InsertBraces: true
https://clang.llvm.org/docs/ClangFormatStyleOptions.html#insertbraces).
This could also have been prevented by stricter tests, adjusted in 0ec0004 |
Thanks. FYI: I tried the new test my unfixed copy and it fails as expected:
(BTW: the issue was reported on the Italian GRASS GIS mailing list) |
@wenzeslaus can we change the Centos7 CI runner to not required? Otherwise we'll not be able to merge this rather important and urgent bug fix... |
Having a failing main and all PRs is bad, too. I suggest reverting the change which caused that and/or removing the CentOS 7 check given how outdated it became and its low usefulness. |
This is not an option as the old runner will go EOL on Sept 11, 2023.
Done in #3141 |
PR #3141 (drop Centos7) has been merged, this PR needs to be rebased to become merge'able. |
0ec0004
to
67f5a42
Compare
* r.watershed: fix streams and basins, improve testsuite
* r.watershed: fix streams and basins, improve testsuite
* r.watershed: fix streams and basins, improve testsuite
The PR #2648 introduced a bug in
r.watershed
such that streams and basins are no longer correctly calculated. This PR fixes calculation of streams and basins inr.watershed
.