-
Notifications
You must be signed in to change notification settings - Fork 25
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
Ci runners #169
Ci runners #169
Conversation
dguittet
commented
May 21, 2024
•
edited
Loading
edited
- Github actions now have all 4 platforms
- Tests now include DView and wexsandbox
- Use wxWidgets binary artifact from LK CI instead of building it
- Update cache of LK and SSC to the matching branch
Patch to develop w ci
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.
All tests are passing - great job!
It would be nice to move to wxWidgets 3.2.4 to be consistent with our current build process and public releases.
Is the change to 3.2.4 easily available for updating as wxWidgets changes?
I see "latest" and "3.2.3" used in the scripts - will the upgrade be as simple as changing to 3.2.4?
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.
Great job with the caching!
It looks like there are a few places where the develop branches of other repos are references, whereas this is being merged into patch. Is there a reason for the mismatch?
.github/workflows/ci.yml
Outdated
echo "RAPIDJSONDIR=$GITHUB_WORKSPACE/ssc" >>$GITHUB_ENV | ||
- name: Get git ref of sibling dependency SSC | ||
run: | | ||
ref=$(git ls-remote --exit-code git://github.com/NREL/ssc.git refs/heads/patch | awk '{print $1}') | ||
ref=$(git ls-remote --exit-code https://github.com/NREL/ssc.git refs/heads/develop | awk '{print $1}') |
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.
This looks like there will be a branch mismatch between patch and develop. Should this be patch?
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.
How is this "base" branch handled? Are there different ci.yml
for each 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.
For events triggered by pull_request
, then you have the pull request merge branch, but for the standalone push
, how should the default branch be determined as develop or patch?
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.
Currently in SAM's patch branch, the base branch is also develop: https://github.com/NREL/SAM/blob/1ed59e9b181cf1f23d7ac1f696b5cb91805c7936/.github/workflows/ci.yml#L70
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.
That's not intentional: NREL/SAM#1664 I'm not sure how that got reverted but it should be patch.
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.
For events triggered by
pull_request
, then you have the pull request merge branch, but for the standalonepush
, how should the default branch be determined as develop or patch?
Branches from patch should have patch as the default in the CI script, whereas branches from develop should have develop.
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.
For events triggered by
pull_request
, then you have the pull request merge branch, but for the standalonepush
, how should the default branch be determined as develop or patch?Branches from patch should have patch as the default in the CI script, whereas branches from develop should have develop.
Yes, that is clear. My question was mechanistic. I may just have two versions of ci.yml on each 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.
Most years, a commit when we switch from patch to develop (and vice versa) has been sufficient, but as we see right now it's error prone. How would two ci.yml instances work?
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.
This might be a bad idea but I was thinking we'd have a different version of ci.yml on each branch. I think then the first time we merge patch into develop, we have to manually unselect the changes in ci.yml. But then after that, that difference won't be a tracked diff, and shouldn't show up. I'm not 100% sure it'll work but I could try.
There kind of is a way to get the "closest parent branch" but it uses a lot of bash magic (that I don't understand) and might be unreliable.
* Ci runners (#169) * Merge in patch branch and re-target develop * add windows build * windows ci * windows * windows * fix paths * windows lk * debug * check python * python version * try downloading wx * update linux * linux * fix syntax * download wx * add secrets * tokens * download_wx * download_wx * token * msg * fix * cache build off * windows * add mac build * fix dirs * fix mac * pip on mac * pip * mac * fix git ls-remote * fix mac * mac * update all * windows secrets * fix paths * add error message * build lk always * fix paths * fix paths * fix paths * windows * move windows download * clean up and fix artifacts * use caches * windows cache * fix caching * fix paths for artifacts * fix caching path * fix caching * get branch name * cleanup * fix ssc cache * fix windows * use caching for wx 3.2.4 * release mode * release ver * fix windows path * remove download_wx.py * add default branch * debug vs rel for MSVC * select branch based on push or pull_request --------- Co-authored-by: Brian Mirletz <brian.mirletz@nrel.gov> * shell * fix * fix windows * windows --------- Co-authored-by: Brian Mirletz <brian.mirletz@nrel.gov>