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

Simplify CLib's (Shared)Cabinet #1780

Merged
merged 3 commits into from
Aug 19, 2024
Merged

Conversation

ischoegl
Copy link
Member

@ischoegl ischoegl commented Aug 18, 2024

Changes proposed in this pull request

This PR is mostly house-keeping for a simple bulk-replacement to streamline CLib's storage class (a left-over item from #1770 which I decided to break down into more manageable chunks)

  • Replace all occurrences of SharedCabinet::item by SharedCabinet::at
  • Replace all occurrences of SharedCabinet::get by SharedCabinet::as`
  • Eliminate SharedCabinet::item and SharedCabinet::get
  • Rename SharedCabinet back to Cabinet (original name of before shared pointers were introduced in Smart clib cabinets / add Solution to clib API #1448)

Checklist

  • The pull request includes a clear description of this code change
  • Commit messages have short titles and reference relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • Style & formatting of contributed code follows contributing guidelines
  • The pull request is ready for review

@ischoegl ischoegl added the clib label Aug 18, 2024
@ischoegl ischoegl marked this pull request as ready for review August 18, 2024 22:35
@ischoegl ischoegl requested a review from a team August 18, 2024 22:35
Copy link

codecov bot commented Aug 18, 2024

Codecov Report

Attention: Patch coverage is 19.87041% with 371 lines in your changes missing coverage. Please review.

Project coverage is 73.20%. Comparing base (74c971d) to head (08c9ee6).
Report is 3 commits behind head on main.

Files Patch % Lines
src/clib/ct.cpp 24.55% 169 Missing ⚠️
src/clib/ctonedim.cpp 26.92% 57 Missing ⚠️
src/clib/ctmultiphase.cpp 1.78% 55 Missing ⚠️
src/clib/ctreactor.cpp 7.14% 52 Missing ⚠️
src/clib/ctrpath.cpp 0.00% 24 Missing ⚠️
src/clib/ctsurf.cpp 0.00% 8 Missing ⚠️
src/clib/Cabinet.h 50.00% 5 Missing ⚠️
src/clib/ctfunc.cpp 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1780   +/-   ##
=======================================
  Coverage   73.20%   73.20%           
=======================================
  Files         381      381           
  Lines       54244    54240    -4     
  Branches     9237     9236    -1     
=======================================
- Hits        39709    39707    -2     
  Misses      11564    11564           
+ Partials     2971     2969    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ischoegl ischoegl mentioned this pull request Aug 18, 2024
5 tasks
Copy link
Member

@speth speth left a comment

Choose a reason for hiding this comment

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

This makes sense to me, @ischoegl. Just one minor suggestion.

src/clib/ct.cpp Outdated Show resolved Hide resolved
Replaced by SharedCabinet::at
Replaced by SharedCabinet::as
Revert to original name after SharedCabinet was introduced in Cantera#1448 and
the original Cabinet was subsequently removed.
@ischoegl
Copy link
Member Author

This makes sense to me, @ischoegl. Just one minor suggestion.

Thanks for the prompt review, @speth! I adopted your suggestion.

@speth speth merged commit c16cbe6 into Cantera:main Aug 19, 2024
51 of 52 checks passed
@ischoegl ischoegl deleted the clib-housekeeping branch August 19, 2024 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants