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

Specify const types to support 32-bit arch #147

Merged
merged 6 commits into from
Aug 2, 2024

Conversation

wolfogre
Copy link
Contributor

@wolfogre wolfogre commented Aug 2, 2024

Fix #146.

On 386 arch, the bits length of the int type is 32, which makes it impossible to store or compare with math.MaxUint32 or 0x8F92EAB1.

Go will implicitly use the int type, like the returned type of len(); And convert untyped const integer to the int type, like the input type of fmt.Errorf().

@wolfogre wolfogre changed the title Specify const types to support the 386 arch Specify const types to support 32-bit arch Aug 2, 2024
Copy link

codecov bot commented Aug 2, 2024

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Project coverage is 77.34%. Comparing base (d37ab8e) to head (301c94b).
Report is 36 commits behind head on main.

Files Patch % Lines
pkg/encoder.go 0.00% 3 Missing and 3 partials ⚠️
pkg/seekable.go 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #147      +/-   ##
==========================================
+ Coverage   76.10%   77.34%   +1.24%     
==========================================
  Files           7        7              
  Lines         477      384      -93     
==========================================
- Hits          363      297      -66     
+ Misses         88       61      -27     
  Partials       26       26              

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

@wolfogre
Copy link
Contributor Author

wolfogre commented Aug 2, 2024

@SaveTheRbtz Maybe we can filter out less commonly used arch and test for main arch. Feel free to update.

@SaveTheRbtz SaveTheRbtz merged commit 42610f0 into SaveTheRbtz:main Aug 2, 2024
8 of 9 checks passed
@SaveTheRbtz
Copy link
Owner

SaveTheRbtz commented Aug 2, 2024

Switched to using the "first class" targets for now:

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.

Support 386 arch
2 participants