-
Notifications
You must be signed in to change notification settings - Fork 391
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(gnovm/pkg/gnolang): add compile time guards to fail on 32-bit architectures #3643
base: master
Are you sure you want to change the base?
feat(gnovm/pkg/gnolang): add compile time guards to fail on 32-bit architectures #3643
Conversation
🛠 PR Checks SummaryAll Automated Checks passed. ✅ Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
Sounds reasonable. As the gno.land validator set will initially be limited, this probably isn't a pressing action. |
b411964
to
e746a78
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
remove: review/triage-pending
flag
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 idea looks good to me, but please check the comments for some improvements.
README.md
Outdated
@@ -116,3 +116,6 @@ repository offers more resources to dig into. We are eager to see your first PR! | |||
* [![Go Reference](https://pkg.go.dev/badge/hey/google)](https://gnolang.github.io/gno/github.com/gnolang/gno.html) \ | |||
(pkg.go.dev will not show our repository as it has a license it doesn't recognise) | |||
</details> | |||
|
|||
## Declarations | |||
* Gno is only available for 64-bit architectures! |
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.
@leohhhn maybe you have a better idea of the right place(s) to add this info?
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.
definitely shouldn't go in README; maybe in pkg/gnolang/ readme.
Also we should update the goreleaser configuration to avoid 32bit targets.
6834ca8
to
426f9b8
Compare
…chitectures This change adds a guard to fail to compile on 32-bit architectures as it is a project wide decision not to support them. This allows the mainnet launch without sweating trying to make a bunch of runtime changes for the gnovm. Updates gnolang#3288
426f9b8
to
870b1b2
Compare
This change adds build constraints so as to panic if built for 32-bit architectures as it is a project wide decision not to support them.
This allows the mainnet launch without sweating trying to make a bunch of runtime changes for the gnovm.
Updates #3288