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

Use Avaje Prisms instead of referencing annotations directly #277

Closed
wants to merge 29 commits into from

Conversation

SentryMan
Copy link
Collaborator

@SentryMan SentryMan commented Feb 7, 2023

For auto requires, you'll need to use the inject maven plugin though.

  • Use Avaje(Hickory) prisms to access annotation information
  • Only service load modules/plugins if inject is on the classpath
  • replace that one Stack with a Deque

takes care of #276

@rbygrave
Copy link
Contributor

rbygrave commented Feb 8, 2023

Just to say I wouldn't invest too much time into this as I'm likely to go the path of using TypeElement / TypeMirror / AnnotationMirror (as I mentioned earlier).

@SentryMan
Copy link
Collaborator Author

I mean I got it working, and it works when I modularize my javalin project. But it is what it is I suppose.

@SentryMan SentryMan closed this Feb 8, 2023
@SentryMan
Copy link
Collaborator Author

Just to say I wouldn't invest too much time into this as I'm likely to go the path of using TypeElement / TypeMirror / AnnotationMirror (as I mentioned earlier).

Sure you won't change your mind? hickory seems pretty rad and easy to use compared to AnnotationMirrors.

@rbygrave
Copy link
Contributor

rbygrave commented Feb 8, 2023

Maybe. I'll put up a branch and we can compare ... noting that in the back of my mind there is:

  • This library is expected to live for a long time / many years longevity / think 15 years
  • I need to 'own' it including dependencies like hickory and I can't assume people will help over the maintenance lifetime

@SentryMan
Copy link
Collaborator Author

SentryMan commented Feb 8, 2023

think 15 years

hmm, I would have thought that number would be higher.

I can't assume people will help over the maintenance lifetime

I like to stay optimistic about these sorts of things. One could even argue that having hickory makes the code easier to parse when compared to Annotation Mirrors and so would make it more likely for people to contribute.

In any case, if it breaks something in the future, my pride won't allow me to leave it broken if I was the one who caused it. I'd be the first to remove it.

@agentgt
Copy link

agentgt commented Feb 8, 2023

@rbygrave Hickory is highly stable but just not supported. It is a very simple project and I was planning on forking it and releasing an updated version to maven central.

It is important to understand that Hickory is never a true dependency. It is a zero dep code generator. The annotations it use are retention = SOURCE. The code it produces has zero references to anything hickory. In fact it inspired me to make my own annotation library produce zero dep code as an option: https://jstach.io/jstachio/#maven_zerodep

MapStruct has gemtools which is a more update to date fork but sadly unlike Hickory it is not zero dep: mapstruct/tools-gem#11

If @SentryMan and I did not use it the code change would not have happened in a couple of hours like we did yesterday.

Of course avaje instead of me could host a fork of hickory and then you would guarantee its future for these libraries. I'm planning to do it for jstachio at some point this month.

@SentryMan
Copy link
Collaborator Author

Looks like prisms are back on the menu boys

@SentryMan SentryMan reopened this Feb 8, 2023
@SentryMan SentryMan changed the title Now we don't need to use annotationProcessorPaths for modular projects Use Avaje Prisms instead of referencing annotations directly Feb 8, 2023
@rbygrave rbygrave added this to the 8.12 milestone Feb 9, 2023
@rbygrave
Copy link
Contributor

rbygrave commented Feb 9, 2023

This was merged in via a squashed commit - 064d8be

@rbygrave
Copy link
Contributor

rbygrave commented Feb 9, 2023

Closing as merged via the squashed commit - 064d8be , these changes will be in 8.12

@rbygrave rbygrave closed this Feb 9, 2023
@SentryMan SentryMan deleted the prism branch February 25, 2023 17:45
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.

None yet

3 participants