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

feat(#770): code() in OpcodeName.java for compatibility with opeo #771

Merged
merged 8 commits into from
Oct 18, 2024

Conversation

h1alexbel
Copy link
Contributor

@h1alexbel h1alexbel commented Oct 18, 2024

In this pull I've added new method to OpcodeName.java: code(). This method is required to be used by OPEO in order to resolve objectionary/opeo-maven-plugin#75.

closes #770
History:


PR-Codex overview

This PR introduces a new method code() in the OpcodeName class and adds a corresponding test in OpcodeNameTest. The method retrieves the opcode value, and the test verifies its functionality.

Detailed summary

  • Added a Javadoc comment for the code() method in OpcodeName.
  • Implemented the code() method to return the opcode value.
  • Created a new test method returnsOpcode() in OpcodeNameTest to validate the code() method's output.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@h1alexbel
Copy link
Contributor Author

@volodya-lombrozo take a look, please

Copy link
Member

@volodya-lombrozo volodya-lombrozo left a comment

Choose a reason for hiding this comment

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

@h1alexbel Thank you for the contribution! Only one small correction is needed.

.code();
MatcherAssert.assertThat(
String.format(
"Opcode code: %s does not matches with expected: %s",
Copy link
Member

Choose a reason for hiding this comment

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

@h1alexbel NIT: it's better to use %d for numbers. Docs.

@h1alexbel
Copy link
Contributor Author

@volodya-lombrozo updated pull request. Take a look again, please

@volodya-lombrozo
Copy link
Member

@h1alexbel Looks good t me, thank you.

@volodya-lombrozo
Copy link
Member

@rultor merge

@rultor
Copy link
Contributor

rultor commented Oct 18, 2024

@rultor merge

@volodya-lombrozo OK, I'll try to merge now. You can check the progress of the merge here.

@rultor rultor merged commit 9a5a3a6 into objectionary:master Oct 18, 2024
11 checks passed
@rultor
Copy link
Contributor

rultor commented Oct 18, 2024

@rultor merge

@volodya-lombrozo Done! FYI, the full log is here (took me 8min).

@h1alexbel
Copy link
Contributor Author

@volodya-lombrozo could you release a new version of JEO?

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.

OpcodeName.java:37-39: Code duplication with... OpcodeName#code for compatibility with opeo
4 participants