-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add support for checking out GitHub repositories #208
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #208 +/- ##
==========================================
- Coverage 85.37% 82.67% -2.71%
==========================================
Files 27 28 +1
Lines 1409 1495 +86
==========================================
+ Hits 1203 1236 +33
- Misses 206 259 +53 ☔ View full report in Codecov by Sentry. |
b76d177
to
6f586a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit puzzled with the test_model.py
. Since we use an undefined MockRepo, we aren't testing if things work for a GitRepo instance which does things a bit differently. Or am I missing something? I'm happy to discuss directly with you if it helps.
realisations: | ||
- repo: | ||
svn: | ||
branch_path: trunk | ||
- repo: | ||
svn: | ||
branch_path: branches/Users/ccc561/v3.0-YP-changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we change this to git? BTW, the svn config isn't valid anymore, I deleted the branches/Users/ccc561/v3.0-YP-changes
branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bschroeter was config-valid.yml example meant to be exhaustive in the config options or just a simple success case to test the schema validation? If the latter then this probably isn't necessary since the config is still valid from a validation perspective?
Yes you are right. I think you are referring to this line: Lines 40 to 41 in 6f586a1
This is why writing tests forces you to write better code 😆. The I'm happy to add a comment here so that this gets addressed. |
Here is an integration that checks out and builds an SVN repo and a Git repo: #!/bin/bash
bench_example_dir='bench_example_test_build_git_and_svn'
rm -rf $bench_example_dir
git clone [email protected]:CABLE-LSM/bench_example.git $bench_example_dir
cd $bench_example_dir
git reset --hard 6287539e96fc8ef36dc578201fbf9847314147fb
cat > config.yaml << EOL
project: tm70
experiment: AU-Tum
realisations:
- repo:
svn:
branch_path: trunk
- repo:
git:
branch: main
modules: [
intel-compiler/2021.1.1,
netcdf/4.7.4,
openmpi/4.1.0
]
EOL
benchcab checkout -v && benchcab build -v
|
Yes, add a comment and an issue and that's good as is for now. |
PR: #208 Co-authored-by: Claire Carouge <[email protected]>
PR: #208 Co-authored-by: Claire Carouge <[email protected]>
PR: #208 Co-authored-by: Claire Carouge <[email protected]>
PR: #208 Co-authored-by: Claire Carouge <[email protected]>
PR: #208 Co-authored-by: Claire Carouge <[email protected]>
PR: #208 Co-authored-by: Claire Carouge <[email protected]>
bf6d8df
to
6e7fde8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go.
Changes part of: CABLE-LSM/benchcab#208
Currently benchcab can only fetch repositories from the subversion repository. This change adds the ability to specify and checkout repositories from GitHub so that benchcab can be used when CABLE moves over to Git.
Fixes #183, #197