Skip to content

Conversation

tjrtfarnham
Copy link
Contributor

Hello @angularsen. Thanks for the help...github is a bit different. Note, these changes generate the testing error I described in the discussions about the two lists of enumerations not being identical since the order of the entries are different from one another. I'm also curious why the generated enum is 42 when it seems like it should have chosen 33?

@tjrtfarnham
Copy link
Contributor Author

@angularsen Hoping you might find some time to have a look at this to see what we can do to remedy the test failure.

@angularsen
Copy link
Owner

Hi yes I hope to get to this in the next few days, just a bit wrapped up in work before the summer holidays.

@angularsen
Copy link
Owner

Just an update, am on a vaction this week and will probably not have time to look before the end of the weekend.

@angularsen angularsen force-pushed the add-mach-speed-unit branch from 5d3656e to 74f9563 Compare July 16, 2022 15:58
angularsen added a commit that referenced this pull request Jul 16, 2022
Test case `{_quantity.Name}_QuantityInfo_ReturnsQuantityInfoDescribingQuantity` failed when adding a new unit in #1104 .

QuantityInfo.Units are ordered lexically, but the test enumerated the enum values that have a different ordering.
@angularsen angularsen force-pushed the add-mach-speed-unit branch from 74f9563 to c85c5d9 Compare July 16, 2022 16:16
Copy link
Owner

@angularsen angularsen left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, work and vacation 🌞

I fixed the test case that you hit, in #1108 , then rebased this PR to bring that fix in.

This is almost ready to merge, just a few minor changes required.

@codecov
Copy link

codecov bot commented Jul 16, 2022

Codecov Report

Merging #1104 (ee75c36) into master (ab063b5) will increase coverage by 0%.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #1104   +/-   ##
======================================
  Coverage      85%     85%           
======================================
  Files         318     318           
  Lines       48795   48805   +10     
======================================
+ Hits        41665   41675   +10     
  Misses       7130    7130           
Impacted Files Coverage Δ
...ensions/GeneratedCode/NumberToSpeedExtensions.g.cs 100% <100%> (ø)
UnitsNet/GeneratedCode/Quantities/Speed.g.cs 90% <100%> (+<1%) ⬆️

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 ab063b5...ee75c36. Read the comment docs.

@angularsen angularsen changed the title Adding Mach speed unit. Standard conversion at 20C and 1atm Add Mach speed unit Jul 22, 2022
@angularsen angularsen merged commit 1aadd0b into angularsen:master Jul 22, 2022
@angularsen
Copy link
Owner

Thanks! The nuget should be out in an hour or so. The builds are a bit slow these days.

Release UnitsNet/4.139.0 · angularsen/UnitsNet

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.

2 participants