-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Refactor windows workflow to work on standard windows #6190
Conversation
Currently the windows workflows require bash installed, which is not standard o windows - not to say unusual to have a linux shell as a windows dependency. it also modifies some syntaxes that seems to fail on windows 11 powershell sessions, like the && Lastly, some git authentication hack was modified as it should not be eeded to perform git submodule update
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6190 +/- ##
==========================================
+ Coverage 56.95% 57.09% +0.14%
==========================================
Files 506 506
Lines 30467 30974 +507
Branches 4592 4603 +11
==========================================
+ Hits 17353 17686 +333
- Misses 12285 12464 +179
+ Partials 829 824 -5 ☔ View full report in Codecov by Sentry. |
.github/workflows/release_win.yml
Outdated
python -m pip install -q onnxruntime==1.17.3 | ||
$Env:ORT_MAX_IR_SUPPORTED_VERSION=9 | ||
$Env:ORT_MAX_ML_OPSET_SUPPORTED_VERSION=3 | ||
$Env:ORT_MAX_ONNX_OPSET_SUPPORTED_VERSION=20 | ||
pytest | ||
|
||
- name: Clean Workspace | ||
if: always() |
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.
Why is this required?
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.
Because it is not always guaranteed to have a clean workspace in all github instances.
One example is github enterprise :)
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.
sorry, somehow i missed the github notifications for this pr
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 see. check if the updated checkout step solves this? If not feel free to add the step back with comments for future readers. Thanks!
Lgtm thanks! One comment to clarify the cleaning step. Not sure why it is needed yet. |
I still think we need to understand why the Clean Workspace step is needed. In theory the checkout workflow should have inserted a clean up step already |
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Currently the windows workflows require bash installed, which is not standard o windows - not to say unusual to have a linux shell as a windows dependency.
it also modifies some syntaxes that seems to fail on windows 11 powershell sessions, like the &&
Lastly, some git authentication hack was modified as it should not be needed to perform git submodule update