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

Thermal conductance #408

Merged
merged 1 commit into from
Feb 10, 2023
Merged

Conversation

calbaker
Copy link
Contributor

@calbaker calbaker commented Feb 8, 2023

I'm pretty sure I got this right. I need thermal conductance [W/K] for a number of projects I'm working on so I thought I'd try a PR. Let me know if anything important needs attention. Please check my work!

@iliekturtles
Copy link
Owner

Thanks for the PR! Changes look really good. I pushed some minor fixes to your branch Mostly formatting. Please review and squash everything down to one commit. Test suite is running now.

@calbaker calbaker force-pushed the thermal-conductance branch from 98a9768 to 8420e1d Compare February 8, 2023 19:36
@calbaker
Copy link
Contributor Author

calbaker commented Feb 8, 2023

Please review and squash everything down to one commit.

@iliekturtles, I think we're all set!

BTW, your user name is great. My wife and I have saved numerous snapping turtles that were crossing roads by helping them across and blocking traffic.

@iliekturtles
Copy link
Owner

iliekturtles commented Feb 9, 2023

Could you also update the commit message on the squashed commit. Something like Add thermal conductance quantity. as the title and a short description of the quantity if desired.

EDIT: I like turtles!

 * units of W / K
 * heat transfer rate per temperature difference

Commit history:
copied from thermal_conductivity.rs

thermal_conductance seems to work.  tests pass

Code review fixes

 * Fix Celsius casing.
 * Formatting.
@calbaker calbaker force-pushed the thermal-conductance branch from 8420e1d to 0f73157 Compare February 9, 2023 14:45
@calbaker
Copy link
Contributor Author

calbaker commented Feb 9, 2023

@iliekturtles , should be all set now!

@iliekturtles iliekturtles merged commit df56da7 into iliekturtles:master Feb 10, 2023
@iliekturtles
Copy link
Owner

Merged. Thanks!

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