Skip to content
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

Add altimeter units and leave unit conversion up to the user #65

Merged
merged 1 commit into from
Feb 3, 2023

Conversation

aeharding
Copy link
Owner

@drmrbrewer and @clioh, if you have a moment I would appreciate a quick review!

Specifically, I wasn't sure whether to specify AltimeterUnit's value as "mb" (millibars) or "hPa".

What do you prefer?

@clioh
Copy link

clioh commented Feb 3, 2023

The PR LGTM.

I don’t work in that part of the world so I’m not sure what the preferred terminology is. Looks like the internet uses them pretty interchanbly so I’d say go for whichever you like more

@drmrbrewer
Copy link

I think that hPa is more common in all the weather-related APIs I have used... in fact I can't remember it ever being documented as mb (not that it would be wrong to do so)... hence hPa is preferable IMHO! Seems like this is as described here too... i.e. hPa preferred though mb not wrong (and still used in some places)... Pa is the SI unit for pressure so it seems somewhat old-fashioned to be referring to millibars.

@aeharding
Copy link
Owner Author

Great, thanks for the feedback! I'll keep it as it is then :)

@aeharding aeharding merged commit 42caa2f into main Feb 3, 2023
@aeharding aeharding deleted the expose-altimeter-units branch February 3, 2023 13:43
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