Skip to content
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

feat: add Win-amd64 profile #1410

Merged
merged 1 commit into from
Feb 18, 2025
Merged

feat: add Win-amd64 profile #1410

merged 1 commit into from
Feb 18, 2025

Conversation

wForget
Copy link
Member

@wForget wForget commented Feb 16, 2025

Which issue does this PR close?

Closes #1409.

Rationale for this change

Support comet on Win-amd64

What changes are included in this PR?

add Win-amd64 profile

How are these changes tested?

Build and run test successfully on windows with amd64 cpu

image

@codecov-commenter
Copy link

codecov-commenter commented Feb 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 39.32%. Comparing base (f09f8af) to head (8b8a4a0).
Report is 37 commits behind head on main.

Additional details and impacted files
@@              Coverage Diff              @@
##               main    #1410       +/-   ##
=============================================
- Coverage     56.12%   39.32%   -16.81%     
- Complexity      976     2081     +1105     
=============================================
  Files           119      265      +146     
  Lines         11743    61132    +49389     
  Branches       2251    12962    +10711     
=============================================
+ Hits           6591    24040    +17449     
- Misses         4012    32587    +28575     
- Partials       1140     4505     +3365     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @wForget. I have no way to test this, but LGTM.

@@ -477,6 +477,20 @@ under the License.
</properties>
</profile>

<profile>
<id>Win-amd64</id>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, we already have Win-x86 but Win-amd64 is different?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they have different cpu architectures. My computer at home is windows with amd 4900 cpu.

@wForget
Copy link
Member Author

wForget commented Feb 18, 2025

Thanks @wForget. I have no way to test this, but LGTM.

@andygrove Thank you. I have verified this locally, I'll provide screenshots later.

@wForget
Copy link
Member Author

wForget commented Feb 18, 2025

Thanks @wForget. I have no way to test this, but LGTM.

@andygrove Thank you. I have verified this locally, I'll provide screenshots later.

I have updated screenshots in pr description

@andygrove
Copy link
Member

I have updated screenshots in pr description

Thanks @wForget

@andygrove andygrove merged commit 322336d into apache:main Feb 18, 2025
76 checks passed
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.

Add Win-amd64 profile
4 participants