Skip to content

docs: make clear that latitude is first on nearbyGPSCoordinate #864

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Apr 22, 2022
Merged

docs: make clear that latitude is first on nearbyGPSCoordinate #864

merged 7 commits into from
Apr 22, 2022

Conversation

efstathiosntonas
Copy link
Contributor

No description provided.

@efstathiosntonas efstathiosntonas requested a review from a team as a code owner April 22, 2022 08:15
@efstathiosntonas efstathiosntonas changed the title make clear that latitude is first on nearbyGPSCoordinate docs: make clear that latitude is first on nearbyGPSCoordinate Apr 22, 2022
ST-DDT
ST-DDT previously approved these changes Apr 22, 2022
View reviewed changes
Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

Just curious, isn't that the standard?

@ST-DDT ST-DDT added c: docs Improvements or additions to documentation p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug labels Apr 22, 2022
@ST-DDT ST-DDT added this to the v6.3 - Next Minor milestone Apr 22, 2022
@ST-DDT ST-DDT requested review from a team April 22, 2022 08:21
@efstathiosntonas
Copy link
Contributor Author

@ST-DDT no, mongodb wants the longitude first. Google maps wants latitude first.

@codecov
Copy link

codecov bot commented Apr 22, 2022 •
edited
Loading

Codecov Report

Merging #864 (bce1f2c) into main (9b0d100) will not change coverage.
The diff coverage is 100.00%.

❗ Current head bce1f2c differs from pull request most recent head 12afbeb. Consider uploading reports for the commit 12afbeb to get more accurate results

@@           Coverage Diff           @@
##             main     #864   +/-   ##
=======================================
  Coverage   99.42%   99.42%           
=======================================
  Files        1958     1958           
  Lines      210773   210773           
  Branches      904      904           
=======================================
  Hits       209557   209557           
  Misses       1159     1159           
  Partials       57       57
Impacted Files Coverage Δ
src/address.ts 99.81% <100.00%> (ø)

@ST-DDT
Copy link
Member

ST-DDT commented Apr 22, 2022

Could you please add it to L31 (coordinateWithOffset()) as well?

@efstathiosntonas
Copy link
Contributor Author

sure, missed that one, sorry.

ST-DDT
ST-DDT previously approved these changes Apr 22, 2022
View reviewed changes
@ST-DDT ST-DDT requested a review from a team April 22, 2022 08:55
Shinigami92
Shinigami92 requested changes Apr 22, 2022
View reviewed changes
Shinigami92
Shinigami92 requested changes Apr 22, 2022
View reviewed changes
Shinigami92
Shinigami92 requested changes Apr 22, 2022
View reviewed changes
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

Also line 43 could be adjusted 🤔

@ST-DDT
Copy link
Member

ST-DDT commented Apr 22, 2022

Seems to work as expected:
grafik

@Shinigami92 Shinigami92 enabled auto-merge (squash) April 22, 2022 11:53
@Shinigami92 Shinigami92 merged commit 606efc5 into faker-js:main Apr 22, 2022
@efstathiosntonas
Copy link
Contributor Author

efstathiosntonas commented Apr 22, 2022 •
edited
Loading

Thank you @ST-DDT @Shinigami92, keep it up! I think we saved some time for the fellow devs in this one, had to go through the source code to see witch came first.

@efstathiosntonas efstathiosntonas deleted the patch-1 branch April 22, 2022 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ST-DDT ST-DDT ST-DDT approved these changes

@Shinigami92 Shinigami92 Shinigami92 approved these changes

Assignees

@efstathiosntonas efstathiosntonas

Labels
c: docs Improvements or additions to documentation p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
None yet
Milestone
v6.3 - Next Minor
Development

Successfully merging this pull request may close these issues.
3 participants
@efstathiosntonas @ST-DDT @Shinigami92