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

feat(volume_rate): Add cubic_meter_per_min and cubic_meter_per_hour as volume rate units #406

Merged
merged 1 commit into from
Feb 11, 2023

Conversation

Uzaaft
Copy link
Contributor

@Uzaaft Uzaaft commented Feb 4, 2023

This PR attempts to add cubic_meter_per_min and cubic_meter_per_hour as volume rate units.

@Uzaaft
Copy link
Contributor Author

Uzaaft commented Feb 4, 2023

Waiting on #405 before marking this as ready for review.a

@Uzaaft Uzaaft changed the title feat: Add ton_per_minute and ton_per_day as volume rate units feat(volume_rate): Add cubic_meter_per_min and cubic_meter_per_hour a… … eaaa8d4 …s volume rate units Feb 4, 2023
@Uzaaft Uzaaft changed the title feat(volume_rate): Add cubic_meter_per_min and cubic_meter_per_hour a… … eaaa8d4 …s volume rate units feat(volume_rate): Add cubic_meter_per_min and cubic_meter_per_hour as volume rate units Feb 4, 2023
Copy link
Owner

@iliekturtles iliekturtles left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Changes look pretty good. Two minor changes below.

src/si/volume_rate.rs Outdated Show resolved Hide resolved
@Uzaaft
Copy link
Contributor Author

Uzaaft commented Feb 4, 2023

Thants for the suggestions, implemented those. :)
While I have you here @iliekturtles . I was wondering if you would be open to implement(or open for a contribution) that adds support for standard and normal conditions to the codebase?
https://en.wikipedia.org/wiki/Standard_temperature_and_pressure

@iliekturtles
Copy link
Owner

Similar to the other PR can you squash the commits and update the commit message. At the same time you can exclude the mass rate changes and just base this on top of iliekturles/master.

I'm definitely open to STP contributions. AmountOfSubstance already has a few STP specific units. Are you thinking about adding more or something different to more generally support STP?

@Uzaaft
Copy link
Contributor Author

Uzaaft commented Feb 11, 2023

Similar to the other PR can you squash the commits and update the commit message. At the same time you can exclude the mass rate changes and just base this on top of iliekturles/master.

I'm definitely open to STP contributions. AmountOfSubstance already has a few STP specific units. Are you thinking about adding more or something different to more generally support STP?

well, what I ideally would want is to have a general support of STP wherever it could apply, as well as adding methods to convert from/into other units. I'll have to take a look at how AmountOfSubstance does it.

@Uzaaft Uzaaft marked this pull request as ready for review February 11, 2023 17:16
@Uzaaft
Copy link
Contributor Author

Uzaaft commented Feb 11, 2023

Everything should be ready now.

@iliekturtles iliekturtles merged commit a7796b6 into iliekturtles:master Feb 11, 2023
@iliekturtles
Copy link
Owner

Thanks for the PR!

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