Skip to content

PB-859: Use the same approach to compute polygon area.#1053

Merged
ismailsunni merged 4 commits intodevelopfrom
pb-869-different-area-calculation
Sep 4, 2024
Merged

PB-859: Use the same approach to compute polygon area.#1053
ismailsunni merged 4 commits intodevelopfrom
pb-869-different-area-calculation

Conversation

@ismailsunni
Copy link
Contributor

@ismailsunni ismailsunni commented Sep 2, 2024

@ismailsunni ismailsunni requested review from ltshb and pakb September 2, 2024 12:15
@cypress
Copy link

cypress bot commented Sep 2, 2024

web-mapviewer    Run #3204

Run Properties:  status check passed Passed #3204  •  git commit fc99fbd801: PB-869: Refactor function to compute area and perimeter using geographic library...
Project web-mapviewer
Branch Review pb-869-different-area-calculation
Run status status check passed Passed #3204
Run duration 05m 38s
Commit git commit fc99fbd801: PB-869: Refactor function to compute area and perimeter using geographic library...
Committer Ismail Sunni
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 21
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 210
View all changes introduced in this branch ↗︎

@ismailsunni ismailsunni force-pushed the pb-869-different-area-calculation branch from 1f2a4f3 to 7192ce1 Compare September 2, 2024 14:45
Copy link
Contributor

@ltshb ltshb left a comment

Choose a reason for hiding this comment

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

Code looks good but the PR and code should be better documented, eg. why using the GeographicLib ? From where come this library etc...

@ismailsunni
Copy link
Contributor Author

@ltshb I don't know either why or when we decide to use GeographicLib in the first time. In this PR I just simply make the area calculated the same way.

Copy link
Contributor

@pakb pakb left a comment

Choose a reason for hiding this comment

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

Might be good to reference that you are copying here what you found out here

or even better, have both location use the same piece of code

@ismailsunni ismailsunni force-pushed the pb-869-different-area-calculation branch from a16abf4 to fc99fbd Compare September 4, 2024 07:46
@ismailsunni
Copy link
Contributor Author

@pakb I did put a reference, but I think it's better to use the same code like you said. I have refactored it.

@ismailsunni ismailsunni merged commit 49dd1b4 into develop Sep 4, 2024
@ismailsunni ismailsunni deleted the pb-869-different-area-calculation branch September 4, 2024 08:51
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.

3 participants