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

[Shell] Deprecate shell setup scripts #1189

Merged
merged 3 commits into from
Feb 8, 2022

Conversation

bryanwweber
Copy link
Member

Related to Cantera/enhancements#135

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

Copy link
Member

@ischoegl ischoegl left a comment

Choose a reason for hiding this comment

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

Minor comment. I think it would make sense to add context for what users should use instead.

platform/posix/setup_cantera.csh.in Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Feb 7, 2022

Codecov Report

Merging #1189 (35eb317) into main (03c6e70) will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1189      +/-   ##
==========================================
+ Coverage   65.39%   65.44%   +0.05%     
==========================================
  Files         318      318              
  Lines       46109    46106       -3     
  Branches    19601    19604       +3     
==========================================
+ Hits        30153    30175      +22     
+ Misses      13452    13435      -17     
+ Partials     2504     2496       -8     
Impacted Files Coverage Δ
include/cantera/kinetics/Arrhenius.h 98.01% <0.00%> (-1.99%) ⬇️
src/kinetics/ReactionData.cpp 85.03% <0.00%> (-1.37%) ⬇️
src/kinetics/Reaction.cpp 80.48% <0.00%> (ø)
src/kinetics/FalloffFactory.cpp 92.85% <0.00%> (ø)
include/cantera/kinetics/FalloffMgr.h 92.85% <0.00%> (ø)
include/cantera/kinetics/RateCoeffMgr.h 100.00% <0.00%> (ø)
include/cantera/kinetics/MultiRateBase.h 100.00% <0.00%> (ø)
include/cantera/kinetics/FalloffFactory.h 90.00% <0.00%> (ø)
src/kinetics/BulkKinetics.cpp 88.70% <0.00%> (+0.18%) ⬆️
include/cantera/kinetics/ReactionRate.h 87.17% <0.00%> (+0.69%) ⬆️
... and 6 more

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 03c6e70...35eb317. Read the comment docs.

@bryanwweber bryanwweber self-assigned this Feb 8, 2022
@bryanwweber bryanwweber merged commit 0119484 into Cantera:main Feb 8, 2022
@bryanwweber bryanwweber deleted the deprecate-shell-setup-scripts branch February 8, 2022 17:19
@ischoegl
Copy link
Member

ischoegl commented Feb 8, 2022

🎉 ... only thing remaining is to add some additional information on Cantera/enhancements#135

@bryanwweber
Copy link
Member Author

Yeah, I don't use those scripts, so I don't have any particular migration advice. I suppose the advice is "add the relevant parts to your .bashrc/.zshrc/..."

@ischoegl
Copy link
Member

ischoegl commented Feb 9, 2022

On a related note, I believe this needs to be updated also (didn't realize this as I rarely ever install and thus never see this message):

cantera/SConstruct

Lines 2018 to 2033 in 5b58690

if os.name != 'nt':
env['setup_cantera'] = pjoin(env['ct_bindir'], 'setup_cantera')
env['setup_cantera_csh'] = pjoin(env['ct_bindir'], 'setup_cantera.csh')
install_message += textwrap.dedent("""
Setup scripts to configure the environment for Cantera are at:
setup script (bash) {setup_cantera!s}
setup script (csh/tcsh) {setup_cantera_csh!s}
It is recommended that you run the script for your shell by typing:
source {setup_cantera!s}
before using Cantera, or else include its contents in your shell login script.
""".format(**env_dict))

@bryanwweber
Copy link
Member Author

@ischoegl Good point, I'll remove that in #1158

@ischoegl ischoegl mentioned this pull request Mar 7, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants