refactor: Allow coordinate parameter to be [string, string]

[samm459]

Hello! This is my first contribution to this great repo 🙏. Hope it helps!

Opted for :
coordinate?: [longitude: number | string, latitude: number | string]
instead of this:
coordinate?: [longitude: number, latitude: number] | [longitude: string, latitude: string]

Made sense to me given my approach coerces both into numbers either way. Hopefully this is what you were looking for.

Thank you!

[xDivisionByZerox]

Can you provide one extra test with mixed ([number, string]) parameters on the coordinate property? Just to ensure that nothing breaks during runtime.

[xDivisionByZerox]

I changed the title to a refactor, hope you don't mind.

[xDivisionByZerox]

Listed as a todo in #1044.

[codecov]

Codecov Report

Merging #1066 (0b3599b) into main (5b3f85a) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1066      +/-   ##
==========================================
- Coverage   99.65%   99.64%   -0.01%     
==========================================
  Files        2135     2135              
  Lines      229134   229134              
  Branches      974      971       -3     
==========================================
- Hits       228348   228329      -19     
- Misses        765      784      +19     
  Partials       21       21              

Impacted Files	Coverage Δ
src/modules/address/index.ts	98.42% <100.00%> (ø)
src/modules/internet/user-agent.ts	86.37% <0.00%> (-6.38%)	⬇️
src/modules/finance/index.ts	100.00% <0.00%> (+0.68%)	⬆️

[samm459]

Thanks for the feedback! Added another loop to the current test which runs the assertions against all 4 possible type combinations. Does this solution work for you?

[ST-DDT]

Looks good to me. Thanks for your contribution!

[ST-DDT]

We should discuss whether we want to support string coordinates at all:

See also:

• feat(location)!: nearbyGPSCoordinate returns number tuple #1061 (comment)
• feat(location)!: latitude/longitude returns number #1064 (comment)

Sorry for the trouble.

[ST-DDT]

Cross referencing related PRs:

• refactor: Allow coordinate parameter to be [string, string] #1066
• feat(location)!: latitude/longitude returns number #1064
• feat(location)!: nearbyGPSCoordinate returns number tuple #1061

[xDivisionByZerox]

@samm459 We had a team meeting today, where we discussed this PR. We came to the conclusion, that we currently don't feel responsible for providing a wide variety of input types. That means that the PR as-is will not be merged. In the #1044 you can see that this todo is updated to "will not fix".

But, this is your first PR in our project we want you to have a positive feeling of achievement.
Since the todo will no longer be fixed it needs to be removed from the code base.
So would you be so kind and revert your changes and then push an additional commit where you remove the comment?

Changed requirements

[samm459]

Sounds good, can do. After giving it some thought I totally agree with the decision.

[xDivisionByZerox]

@samm459 are you still willing to update your PR? Just wanted to make sure, was just checking open PRs.
You are allowed to take your time.

[ST-DDT]

Superseded by #1234