-
Notifications
You must be signed in to change notification settings - Fork 735
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
Feature: Instruction can read the script number #2081
Feature: Instruction can read the script number #2081
Conversation
Looks good! But we have an existing Could you also add a unit test for a couple negative zeros :}? |
6b2062c
to
c7ff61d
Compare
Ok, c7ff61d072d35562fe15850b4f9631e08afdc2e6 looks good now! But what do you think of renaming |
Should I expose it publicly? In which case, we'd still want the > 4 byte check for both. |
@junderw yeah, I think it should be exposed publicly. |
4dffe7c
to
3bdef42
Compare
Should be good for re-review now. |
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.
ACK 3bdef424f4e6aeb3f2ea8859b26f984148ba34e5
3bdef42
to
767b94e
Compare
If tests pass, this is ready for re-review |
Ouch, CI fail is linter because Scond time this week my review suggestions have failed to build, bad robot. |
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.
ACK 767b94e48dd2dfaaf21c598ec9fe395014c66c04
Thanks for working in my suggestions, appreciate the extra effort.
767b94e
to
cd15c74
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.
ACK cd15c74
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.
ACK cd15c74
Nice, this is useful!
You mean lint* not build ;) |
This new method on
Instruction
will allow for interpreting the Instruction as a script number (i64)