Skip to content

[Shell] Deprecate shell setup scripts#1189

Merged
bryanwweber merged 3 commits intoCantera:mainfrom
bryanwweber:deprecate-shell-setup-scripts
Feb 8, 2022
Merged

[Shell] Deprecate shell setup scripts#1189
bryanwweber merged 3 commits intoCantera:mainfrom
bryanwweber:deprecate-shell-setup-scripts

Conversation

@bryanwweber
Copy link
Copy Markdown
Member

@bryanwweber bryanwweber commented Feb 7, 2022

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

@bryanwweber bryanwweber requested a review from a team February 7, 2022 15:44
ischoegl
ischoegl reviewed Feb 7, 2022
View reviewed changes
Copy link
Copy Markdown
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.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 7, 2022 •
edited
Loading

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
Copy Markdown
Member

ischoegl commented Feb 8, 2022

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

@bryanwweber
Copy link
Copy Markdown
Member Author

bryanwweber commented Feb 8, 2022

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
Copy Markdown
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
Copy Markdown
Member Author

bryanwweber commented Feb 9, 2022

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

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

Reviewers

@ischoegl ischoegl ischoegl approved these changes

Assignees

@bryanwweber bryanwweber

Labels

Linux Low Priority macOS

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bryanwweber @ischoegl