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 units to ElectricCurrentGradient, PressureChangeRate #1274

Conversation

vKaras1337
Copy link
Contributor

@vKaras1337 vKaras1337 commented Jun 29, 2023

Hi
I added some units witch we missed in our hydraulic use cases ;)
and did 2 small fixes? please look over these!

I hope I'm following all the guidelines correctly, if not please advice for changes ;)

Added Units

ElectricCurrentGradient:

  • AmperesPerMinute
  • MilliAmperesPerMinute
  • MilliAmperesPerSecond

I don't know russian, but i copy pasted the unit abbreviation from the other units. Google says its ok.

PressureChangeRate:

  • BarsPerSecond
  • BarsPerMinute
  • MillibarsPerSecond
  • MillibarsPerMinute

Added operators

  • Multiplication of ElectricCurrentGradient and TimeSpan|Duration returns ElectricCurrent
  • Multiplcation of PressureChangeRate and TimeSpan|Duration returns Pressure
  • Division of ElectricCurrent by Time|Duration returns ElectricCurrentGradient
  • Division of Pressure by TimeSpan|Duration returns PressureChangeRate

found some operator only use Duration and some use both TimeSpan and Duration, thats what I went with.

FIX

All generated files in UnitsNet/GeneratedCode/Quanities/ had a change of 1 spacechar.
That's the change in QuantityGenerator.cs.

I set SupportsSIUnitSystem to true in ElectricCurrentGradientTests and added the BaseUnits element on AmperesPerSecond in ElectricCurrentGradient.json.
I think thats correct?

Addidionaly added some generated files that had a change of spaces in the <summary> tag.

@vKaras1337
Copy link
Contributor Author

Ok, that tests worked here and shouldn't be from my changes?
I'll have a look at it!

@angularsen
Copy link
Owner

@vKaras1337 Thanks, this looks like a bunch of great work. Will review shortly.

Broken tests and whitespace were existing issues due to some recent work. I'll look at it.

Whitespace fixed:
#1282

@angularsen
Copy link
Owner

@vKaras1337 I think the broken tests are fixed now

#1267

As Bar is commonly used for hydraulics and I could really use it to display pressure changes in our 'normal' unit.
I don't know any russian, but i copy&pasted the abbreviations from the other units, verification needed?

Added 'Milli' to AmperePerSecond and added AmperePerMinute with 'Milli' to ElectricCurrentGradient to display some slower and minor changes in current.
Added the missing BaseUnits in AmperePerSecond and enabled SupportSIUnitSystem in the tests. I hope that is Ok?
(As it uses Ampere and Second it should as of my understanding of UnitsNet)
…eGen change?

can't find it because it looks like that this line changes has been excluded from commits from atleast  (angularsen#656)
…rrent

and PressureChangeRate and Time to Pressure
@angularsen angularsen force-pushed the add_PressureChange_bar_And_ElectricCurrentGradient_mA branch from f233b51 to cf1b16c Compare July 11, 2023 01:20
@angularsen angularsen changed the title Add BarsPerSecond to PressureChangeRate and more. Add units to ElectricCurrentGradient, PressureChangeRate Jul 11, 2023
@angularsen angularsen merged commit 8ddaf50 into angularsen:master Jul 11, 2023
@angularsen
Copy link
Owner

@angularsen
Copy link
Owner

Nice job, looked perfect to me and a ton of new stuff. Nuget is out 👍

@vKaras1337 vKaras1337 deleted the add_PressureChange_bar_And_ElectricCurrentGradient_mA branch July 21, 2023 08:55
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