Update abbreviations

[taoxlei]

No description provided.

[codecov-io]

Codecov Report

Merging #691 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #691   +/-   ##
=======================================
  Coverage   58.34%   58.34%           
=======================================
  Files         165      165           
  Lines       37450    37450           
=======================================
  Hits        21852    21852           
  Misses      15598    15598

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a27b2a3...16acac1. Read the comment docs.

[tmilnthorp]

While this is a good change, it is a breaking one.

I think that's easily remedied by adding the new abbreviations to the list rather than replacing them (like you did for VolumeFlow).

[taoxlei]

@tmilnthorp You got me. I completely neglected the PR is breaking.

Some of my thoughts for my PR:
Aadding new abbreviation to VolumeFlow is on purpose while the LPS, LPM, LPH are all OK as I see.
But other three commits are really supposed to be some corrections:

• "Abbreviations": [ "PaS" ] --> "Abbreviations": [ "Pa·s" ]: a lowercase 's' usually for second. '·' for multiplication
• "Abbreviations": [ "g/S" ] --> "Abbreviations": [ "g/s" ]: a lowercase 's' usually for second
• "Abbreviations": [ "Btu/hr" ] --> "Abbreviations": [ "Btu/h" ]: here "hr" is a exception while other units all use "h" in UnitsNet. and "h" is a usual abbreviation for hour

Anyhow, I aggree with you that adding abbrevations to the list is reasonable for avoiding breaking changes.

[tmilnthorp]

If you add the abbreviation to the beginning, it should make it the new default. That would be a behavior change, but at least is would still allow parsing of the old units. @angularsen what are you thoughts on this kind of behavior change?

I believe it should be ok as these changes are more standard. It would only break applications that do something with the default abbreviation (say parsing it themselves for whatever reason). But that we can't help. Any code doing round-trip ToString and Parse within UnitsNet will still work so long as the old abbreviations are kept at the end of the list.

[angularsen]

@tmilnthorp @lingyunyumo Yup, I'm fine with changing default abbreviation. I don't consider that a breaking change. Consumers should not depend on ToString() never changing in my opinion, we have unit enums they can rely on instead.

[taoxlei]

@angularsen I re-pushed some commits to this PR. Could you have a look if this PR can be merged?

[tmilnthorp]

Looks good to me, although it looks like you did not run the code generator. The build does not automatically do that and commit the generated files, so it must still be done manually

[angularsen]

I just merged this anyway, will fix on master.

[angularsen]

Release UnitsNet/4.35.0 · angularsen/UnitsNet

[tmilnthorp]

Sounds good. I wonder if we could set up the builds to automatically commit generated code to the source branch if any were found.

[angularsen]

It should be possible by doing a local git diff on the build agent. Would be a nice addition.