-
Notifications
You must be signed in to change notification settings - Fork 3
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
chore: add support for i386 #52
Conversation
206fddd
to
62e7325
Compare
2d5a971
to
215912a
Compare
782f832
to
959086c
Compare
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.
LGTM
@@ -2,7 +2,7 @@ import std/math | |||
import pkg/stew/endians2 | |||
|
|||
type | |||
VarIntCompatible* = range[0..2^62-1] | |||
VarIntCompatible* = range[0'i64..2'i64^62-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.
we generally avoid ranges: https://status-im.github.io/nim-style-guide/language.range.html
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.
It could be addressed in a new PR. What would you recommend in this case?
This PR reuses the
install_nim
action fromnim-libp2p
to install Nimi386
. We aren't running the test oni386
- as GitHub runners are amd64, but only using Nimi386
.It also uses
int64
instead ofint
forVarIntCompatible
andPacketNumber
to supporti386
.