-
Notifications
You must be signed in to change notification settings - Fork 5
Build analytics-cli for Windows x64 #822
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
Conversation
|
😎 Merged successfully - details. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #822 +/- ##
==========================================
+ Coverage 73.58% 73.81% +0.22%
==========================================
Files 72 72
Lines 16168 16168
==========================================
+ Hits 11898 11935 +37
+ Misses 4270 4233 -37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Eli Schleifer <[email protected]>
Signed-off-by: Eli Schleifer <[email protected]>
|
Ok - everything is building now and smoke tests are running. There is no special code perse to support windows besides the build structure here to make it build. I think with the caveat that windows will be experimental for now. We should be able to at least safely land this and cut a pre-release for customer testing |
.github/workflows/release.yml
Outdated
| - name: Upload Bundle CLI artifact (Windows) | ||
| uses: actions/upload-artifact@v4 | ||
| if: matrix.platform.target != 'x86_64-unknown-illumos' | ||
| if: ${{ matrix.platform.target != 'x86_64-unknown-illumos' && contains(matrix.platform.os-name, 'windows') }} |
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.
Since we are already checking for windows, the illumos check here is redundant.
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 catch
.github/workflows/pull_request.yml
Outdated
| platform: | ||
| - os-name: linux-x86_64 | ||
| runs-on: ubuntu-latest | ||
| runs-on: public-amd64-4xlarge-dind-germany |
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.
public runners are free in this repo and require less support from platform (we've had a few issues pop up with internal runners because of incompatible changes). Why make this change?
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.
the GH supplied runners run out of disk space no matter which way I sliced it. If we need larger runners - we have to pay for that. We do use these in other places - so felt reasonable to take that on.
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.
We explicitly removed them from this repo because of various issues we kept hitting and limited internal support. We can add them back in temporarily but it'll require more support from platform is issues do pop up again. @pat-trunk-io for viz.
| target: aarch64-apple-darwin | ||
|
|
||
| - os-name: windows-x86_64 | ||
| runs-on: public-amd64-4xlarge-dind-germany |
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.
same runner question here
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.
same
| artifact-name: aarch64-apple-darwin | ||
|
|
||
| - os-name: windows-x86_64 | ||
| runs-on: public-amd64-4xlarge-dind-germany |
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.
same runner question
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.
same
|
added -experimental to the release .zip naming structure. |
gnalh
left a comment
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.
Approving, but support will be limited for this functionality.
Smoke Tests passing on all platforms (including windows) :
https://github.com/trunk-io/analytics-cli/actions/runs/19517735811