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(#701): Refactor Annotation Properties #739

Merged

Conversation

volodya-lombrozo
Copy link
Member

@volodya-lombrozo volodya-lombrozo commented Sep 30, 2024

In this PR I significantly simplified the work with annotation values.
Now we don't have ugly enums and for each case we have a separate small object that is resposible
only for its own area.

I removed DirectivesAnnotationProperty and BytecodeAnnotationProperty classes since they violate code standards.

Closes: #701.
History:


PR-Codex overview

This PR focuses on refactoring the BytecodeAnnotationProperty class and related components in the org.eolang.jeo.representation package. It introduces new classes for different annotation value types and updates existing logic to utilize these new structures.

Detailed summary

  • Deleted BytecodeAnnotationProperty and DirectivesAnnotationProperty classes.
  • Introduced BytecodePlainAnnotationValue, BytecodeArrayAnnotationValue, BytecodeEnumAnnotationValue, and BytecodeAnnotationAnnotationValue.
  • Updated XmlAnnotationProperty to return new bytecode types.
  • Modified annotationProperty method to use new classes.
  • Added tests for DirectivesPlainAnnotationValue, DirectivesEnumAnnotationValue, DirectivesArrayAnnotationValue, and DirectivesAnnotationAnnotationValue.

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

@volodya-lombrozo
Copy link
Member Author

@rultor merge

@rultor
Copy link
Contributor

rultor commented Sep 30, 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 322ab58 into objectionary:master Sep 30, 2024
11 checks passed
@rultor
Copy link
Contributor

rultor commented Sep 30, 2024

@rultor merge

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

@0crat
Copy link

0crat commented Sep 30, 2024

@volodya-lombrozo Hey there! 👋 Thanks for your contribution! While it's impressive you wrote 1808 hits-of-code, that's actually a bit too much for our bonus system. 😅 We couldn't award points for the code review or comments since there weren't any. But don't worry, we've still got you covered with 4 points! Remember, quality over quantity is key. Keep the contributions coming, but maybe aim for smaller, more focused chunks next time. Your balance is now at +100. Looking forward to your next contribution! 🚀

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.

DirectivesAnnotationProperty.java:38-41: Refactor {@link...
4 participants