Skip to content
This repository has been archived by the owner on Jan 8, 2025. It is now read-only.

implement 0x17 - OR opcode #12 #292

Merged

Conversation

Noeljarillo
Copy link
Contributor

Added OR opcode

Enhancement OpCode

Please check the type of change your PR introduces:

  • Bugfix
  • [x ] Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build-related changes
  • Documentation content changes
  • Other (please describe):

@enitrat
Copy link
Contributor

enitrat commented Sep 6, 2023

Hey Noel, nice to see you here taking some initiatives!

Before opening PRs, could you make sure that there is an existing issue and that you can ask to be assigned on it? This makes the open contributions easier to manage :)

If the issue doesn't exist yet, you can go to https://evm.codes, and copy-paste the corresponding opcode to the issue (you can copy the HTML code of the block to get a nice formatting)

After that, when you open a PR, you need to link the issue with the syntax Closes #issue-number. Thanks!

Copy link
Contributor

@enitrat enitrat left a comment

Choose a reason for hiding this comment

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

Can you add a testcase with values more complex than 0 and 1 ?

e.g from the legacy implementation: 0x89 | 0xC5 = 0xCD

Copy link
Contributor

@enitrat enitrat left a comment

Choose a reason for hiding this comment

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

lgtm 🔥

@enitrat enitrat added this pull request to the merge queue Sep 8, 2023
Merged via the queue into kkrt-labs:main with commit d2b2870 Sep 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants