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

Migrate additional website content #1657

Merged
merged 14 commits into from
Jan 16, 2024
Merged

Migrate additional website content #1657

merged 14 commits into from
Jan 16, 2024

Conversation

speth
Copy link
Member

@speth speth commented Jan 11, 2024

Changes proposed in this pull request

This PR transfers most of the remaining version-specific content from the cantera-website into this repository. Specifically, this includes:

  • Release notes
  • Installation instructions
  • C++ compilation and application tutorials
  • Glossary

In addition, a new page has been added to the "Develop" section explaining a bit about how the source material for the documentation is organized and some useful syntax for working with MyST, reST, and Doxygen.

If applicable, fill in the issue number this pull request is fixing

Continues work on Cantera/enhancements#178

If applicable, provide an example illustrating new features this pull request is introducing

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 previously approved these changes Jan 12, 2024
Copy link
Member

@bryanwweber bryanwweber left a comment

Choose a reason for hiding this comment

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

A few suggestions, but lgtm! Merge whenever you want

doc/doxygen/mainpage.md Outdated Show resolved Hide resolved
doc/doxygen/mainpage.md Outdated Show resolved Hide resolved
doc/sphinx/develop/doc-formatting.md Outdated Show resolved Hide resolved
doc/sphinx/develop/doc-formatting.md Outdated Show resolved Hide resolved
doc/sphinx/install/pip.md Outdated Show resolved Hide resolved
doc/sphinx/install/ubuntu.md Outdated Show resolved Hide resolved
doc/sphinx/reference/glossary.md Outdated Show resolved Hide resolved
doc/sphinx/reference/thermo/index.md Outdated Show resolved Hide resolved
doc/sphinx/userguide/compiling-cxx.md Outdated Show resolved Hide resolved
doc/sphinx/userguide/compiling-cxx.md Outdated Show resolved Hide resolved
Co-authored-by: Bryan Weber <bryan.w.weber@gmail.com>
@speth
Copy link
Member Author

speth commented Jan 13, 2024

Thanks for looking this over, @bryanwweber. I made the suggested changes, which requires re-approval before I can merge.

Copy link

codecov bot commented Jan 13, 2024

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (fb53e96) 72.74% compared to head (6a056b0) 72.75%.
Report is 2 commits behind head on main.

Files Patch % Lines
doc/sphinx/userguide/demo1a.cpp 69.23% 4 Missing ⚠️
doc/sphinx/userguide/demoequil.cpp 71.42% 4 Missing ⚠️
doc/sphinx/userguide/kinetics_transport.cpp 87.87% 4 Missing ⚠️
doc/sphinx/userguide/thermodemo.cpp 83.33% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1657      +/-   ##
==========================================
+ Coverage   72.74%   72.75%   +0.01%     
==========================================
  Files         371      375       +4     
  Lines       56500    56584      +84     
  Branches    20443    20494      +51     
==========================================
+ Hits        41100    41168      +68     
- Misses      12385    12401      +16     
  Partials     3015     3015              

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

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.

@speth ... thank you for your continued efforts on this.

I mainly have a comment on the location of the demo C++ files. I believe that they should be moved to samples/cxx so they are covered by CI, which will ensure that they are kept up to date. Whether this is done in this PR or a subsequent one is inconsequential, as long as it is taken care of before the release of 3.1.

Copy link
Member

Choose a reason for hiding this comment

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

I believe the demo C++ files should be moved to samples/cxx, so they can be part of CI?

Same for all other cpp files in this folder.

doc/sphinx/userguide/python-tutorial.md Show resolved Hide resolved
bryanwweber
bryanwweber previously approved these changes Jan 14, 2024
Copy link
Member

@bryanwweber bryanwweber left a comment

Choose a reason for hiding this comment

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

Thanks @speth!

ischoegl
ischoegl previously approved these changes Jan 16, 2024
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.

Thanks, @speth, feel free to merge. My remaining comment can be easily taken care of in a follow-up PR.

@speth speth dismissed stale reviews from ischoegl and bryanwweber via 6a056b0 January 16, 2024 20:33
@speth
Copy link
Member Author

speth commented Jan 16, 2024

I mainly have a comment on the location of the demo C++ files. I believe that they should be moved to samples/cxx so they are covered by CI, which will ensure that they are kept up to date. Whether this is done in this PR or a subsequent one is inconsequential, as long as it is taken care of before the release of 3.1.

I was planning on setting these up to run as part of the test suite but forgot before opening this PR. So, rather than wait and risk forgetting to ever do this, it is now implemented. I went with an option that's slightly different from your suggestion, since I only want to include these samples in the C++ Tutorial and not have them repeated in the Examples section, where I think they would distract from the more interesting example programs.

@speth speth merged commit 7efb996 into Cantera:main Jan 16, 2024
48 of 49 checks passed
@speth speth deleted the website-transfer branch January 16, 2024 22:17
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.

3 participants