Skip to content

Fix Loop contains which contained a missing negation and add test cases. #156

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

Merged
merged 30 commits into from
May 1, 2025

Conversation

rsned
Copy link
Collaborator

@rsned rsned commented Apr 17, 2025

Loop's hasCrossingRelation() had the wrong condition on one branch. This has been resolved and test cases added ensuring it was fixed.

Fix #77
Fix #78

rsned and others added 28 commits April 2, 2025 01:28
Add the action and set up an initial configuration that passes on the codebase at this point in time.
Invalid Go toolchain version (1 result)
    * go.mod#L0C0:0: As of Go 1.21, toolchain versions [must use the 1.N.P syntax](https://go.dev/doc/toolchain#version).

      `1.21` in `go.mod` does not match this syntax and there is no additional `toolchain` directive, which may cause some `go` commands to fail.
(with comments on what the hash is representing
Add some helpers to shape index cell and range iterator to match C++.
Update loop to use the helpers to more closely match the code flows.
Negate a boolean check that was missed.

TODO: test cases to prove it works now but not before the fix.
@alan-strohm alan-strohm self-assigned this Apr 22, 2025
@panmari
Copy link
Collaborator

panmari commented Apr 28, 2025

@rsned do you need help with addressing the comments? Anything I can assist with?

Rename parameter to be clearer.
Utilize new helper methods to simplify some calls.
Update regression case comments.
@rsned
Copy link
Collaborator Author

rsned commented Apr 28, 2025

Sorry, I missed seeing the notification last week of your comments.

Copy link
Collaborator

@alan-strohm alan-strohm left a comment

Choose a reason for hiding this comment

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

Thanks @rsned! The next release of the Java version will also have the improved test coverage from this pull request (and there's a bug to include it in the C++ version too). This is proof that external contributors improving the go version can lead to benefits for the other languages too!

@alan-strohm alan-strohm merged commit fa1e883 into golang:master May 1, 2025
3 checks passed
abhinavdangeti pushed a commit to blevesearch/geo that referenced this pull request May 8, 2025
…es. (golang#156)

Loop's hasCrossingRelation() had the wrong condition on one branch. This
has been resolved and test cases added ensuring it was fixed.

Fix golang#77
Fix golang#78

---------

Co-authored-by: Alan Strohm <[email protected]>
abhinavdangeti added a commit to blevesearch/geo that referenced this pull request May 12, 2025
Brings in all commits as of May 05, 2025:
```
* 4cab8bf dependabot[bot] | build(deps): bump golangci/golangci-lint-action from 7.0.0 to 8.0.0 and golangci-lint from v2.0 to v2.1 (golang#173)
* c498f45 Robert Snedegar | builder_snapper: Update constants and comments (golang#172)
* 18783dd Robert Snedegar | Update README.md and add CONTRIBUTING.md (golang#171)
* 9edba9f dependabot[bot] | build(deps): bump github/codeql-action from 3.28.16 to 3.28.17 (golang#170)
* 686492e Jesse Rosenstock | Test with 1.21 instead of 1.21.0 (golang#169)
* 29adb2f Panmari | Run tests on a matrix of go versions (golang#168)
* 6580a94 Robert Snedegar | Add missing methods on Edge (golang#166)
* 0738b43 Robert Snedegar | Fix Loop contains  which contained a missing negation and add test cases. (golang#156)
* a9f12e8 dependabot[bot] | build(deps): bump github/codeql-action from 3.28.15 to 3.28.16 (golang#164)
* 8f26a2e Alan Strohm | go.yml: use stable go-version (golang#162)
* 7fd717b cui fliter | all: fix some comments (golang#161)
* 8769abd Robert Snedegar | Update scorecard.yml with full version comment on Upload to code-scan… (golang#160)
* 5905ce1 dependabot[bot] | build(deps): bump github/codeql-action from 3.28.13 to 3.28.15 (golang#159)
* a33ded6 dependabot[bot] | build(deps): bump actions/upload-artifact from 4.6.1 to 4.6.2 (golang#158)
* 5859d1c StepSecurity Bot | [StepSecurity] Apply security best practices (golang#155)
* b9a2fb8 Robert Snedegar | Fix identified linter errors preventing errorlint, exhaustive, inamedparam, and staticcheck from running and enable those checks. (golang#153)
```

---------

Signed-off-by: StepSecurity Bot <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: cuishuang <[email protected]>
Co-authored-by: Robert Snedegar <[email protected]>
Co-authored-by: Alan Strohm <[email protected]>
Co-authored-by: StepSecurity Bot <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: cui fliter <[email protected]>
Co-authored-by: Panmari <[email protected]>
Co-authored-by: Jesse Rosenstock <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug on Polygon Contains (linked to https://github.com/golang/geo/issues/77 ?) Porting bug
3 participants