search

Cantera / cantera-website

Official Cantera Website
https://cantera.org
Other
14 stars 26 forks source link

Update C++ tutorial #192

Closed ischoegl closed 2 years ago

ischoegl commented 2 years ago

Closes #191

ischoegl commented 2 years ago

@speth ... ended up replacing newPhase in the remaining tutorials also, and fixed a couple of other things that were not up-to-date in the documentation. I tested compilation of all examples, and things work as expected.

ischoegl commented 2 years ago

Fwiw, some of the includes can be simplified if Cantera/cantera#1238 were to be adopted.

bryanwweber commented 2 years ago

As @speth mentioned elsewhere, I don't think now is a good time to get into a conversation about the content of the include files. I'd prefer not dropping the lowercase include files from the examples.

I think we should limit this PR to the changes in the export and library configuration, and defer changing the examples until later. Since the website can be updated independently of a release, we can change these to newSolution later.

ischoegl commented 2 years ago

@bryanwweber ... thank you for the comments.

As @speth mentioned elsewhere, I don't think now is a good time to get into a conversation about the content of the include files. I'd prefer not dropping the lowercase include files from the examples.

I'm totally fine with keeping 'convenience' headers. At the same time, not having Solution.h covered is not great.

I think we should limit this PR to the changes in the export and library configuration, and defer changing the examples until later. Since the website can be updated independently of a release, we can change these to newSolution later.

I'm not sure I agree here: newSolution pretty much replaced most of the instances in the C++ test suite already, and is overall much more convenient for the users to use (see factory_demo.cpp!).

My preferred solution would be to switch the imports to

#include "cantera/core.h"

as proposed in PR Cantera/cantera#1238 (will wait on feedback there).

PS: If Cantera/cantera#1238 goes ahead, I'd redirect this PR to testing.

ischoegl commented 2 years ago

@bryanwweber and @speth ... now that cantera/core.h is deferred (PR Cantera/cantera#1238), I updated the examples so they are up to date for the 2.6 release (and also work for 2.5.1). This is ready for another round of reviews.