Skip to content

Update data for Cantera/cantera#1951#4

Closed
ischoegl wants to merge 4 commits intomainfrom
add-license
Closed

Update data for Cantera/cantera#1951#4
ischoegl wants to merge 4 commits intomainfrom
add-license

Conversation

@ischoegl
Copy link
Member

@ischoegl ischoegl commented Aug 23, 2025

  • clarify attribution of mechanisms
  • clarify instructions for CI trigger in Linter action within main repo
  • update file name silicon_carbide.yaml to make the naming convention consistent within this repo: all others use kebab-style.

References Cantera/cantera#1951

Note: earlier versions of this PR included a new license file, but I decided to defer this addition and focus on clarifying the README.md file instead.

@speth
Copy link
Member

speth commented Aug 23, 2025

You want to put the reference to the main repo PR in the title to trigger the CI check there. Which I realize is not clear from the README at present.

@ischoegl ischoegl changed the title Add license Add license (Cantera/cantera#1950) Aug 23, 2025
@ischoegl ischoegl changed the title Add license (Cantera/cantera#1950) Add license and update data for Cantera/cantera#1950 Aug 23, 2025
@ischoegl ischoegl changed the title Add license and update data for Cantera/cantera#1950 Add license and update data for Cantera/cantera#1951 Aug 23, 2025
@ischoegl ischoegl changed the title Add license and update data for Cantera/cantera#1951 Update data for Cantera/cantera#1951 Aug 23, 2025
@ischoegl ischoegl marked this pull request as draft August 23, 2025 21:03
@ischoegl
Copy link
Member Author

ischoegl commented Aug 23, 2025

@speth ... I decided against moving on the license file, and focus on updates of instructions instead. The procedure in #3 was not clear, and I don't think that there's a way for the CI to pull in the updated input file (which only exist in a user's personal fork) into the submodule as previously described (the Linter action works, but the example runners fail as they are not aware of the input file).

@ischoegl ischoegl marked this pull request as ready for review August 23, 2025 21:22
@ischoegl ischoegl force-pushed the add-license branch 4 times, most recently from cd8ff4e to 37af11f Compare August 23, 2025 21:48
@ischoegl ischoegl requested a review from a team August 23, 2025 22:10
@speth
Copy link
Member

speth commented Aug 24, 2025

The procedure in #3 was not clear, and I don't think that there's a way for the CI to pull in the updated input file (which only exist in a user's personal fork) into the submodule as previously described (the Linter action works, but the example runners fail as they are not aware of the input file).

The CI on Cantera/cantera pulls in the updated input files from the PR to this repository using the GitHub CLI, which can figure out the correct fork to fetch from. You can see where this happens in a run for Cantera/cantera#1887 here: https://github.com/Cantera/cantera/actions/runs/17043712679/job/48351760292. None of the cases that run the examples nor the one that builds the docs would have succeeded if that wasn't the case.

I think the reason it doesn't work here is because the checkout of Cantera/cantera-example-data is "shallow". I get the same error as the CI here (fatal: cannot set up tracking information; starting point 'origin/add-license' is not a branch) if I try the following commands locally:

git clone https://github.com/cantera/cantera-example-data --depth 1 example-data-test
cd example-data-test
gh pr checkout --repo Cantera/cantera-example-data 4

whereas without the --depth 1 option it works.

I think this would work if you opened the PR from your fork of cantera-example-data.

@ischoegl
Copy link
Member Author

ischoegl commented Aug 24, 2025

I think this would work if you opened the PR from your fork of cantera-example-data.

Ah - looks like I forgot to fork this! I apparently clarified this in the instructions despite failing to do this myself. Let me try again ... PS: reopened as #5.

@ischoegl ischoegl closed this Aug 24, 2025
@ischoegl ischoegl deleted the add-license branch August 24, 2025 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants