Skip to content
This repository has been archived by the owner on Mar 27, 2024. It is now read-only.

Annotation Extraction #474

Merged
merged 72 commits into from Oct 23, 2020
Merged

Annotation Extraction #474

merged 72 commits into from Oct 23, 2020

Conversation

dbluhm
Copy link

@dbluhm dbluhm commented Oct 12, 2020

This PR builds on top of Michael's changes for annotation extraction. This represents an attempt to simplify and preserve type safety while also clearly defining roles of different aspects of the code.

Notably, this PR introduces the AnnotationExtractor and GeneratedFileWriter interfaces and reworks the WriterGenerator interface to clearly define what kind of data is extracted and processed.

Open to feedback 🙂

walshmm and others added 30 commits July 10, 2020 14:16
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
walshmm/feature/DataModel-annotation

Conflicts:
	org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/Persisted.java
	org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/processors/AnnotatedElement.java
	org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/processors/DataElementProcessor.java
	org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/processors/DataElementSpec.java
	org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/processors/DataFieldSpec.java
	org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/processors/Field.java

Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
walshmm/feature/DataModel-annotation

Conflicts:
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations.proxytest/pom.xml

Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
walshmm/feature/DataModel-annotation

Conflicts:
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/processors/DataElementProcessor.java
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations/src/main/resources/templates/PersistenceHandler.vm

Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
walshmm/feature/ice-annotation-extraction-service

Conflicts:
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/processors/DataElementProcessor.java
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/processors/PersistenceHandlerWriter.java
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations/src/main/resources/templates/PersistenceHandler.vm
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations/src/test/java/org/eclipse/ice/tests/dev/annotations/processors/DataElementProcessorTest.java

Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Daniel Bluhm <bluhmdj@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Daniel Bluhm <bluhmdj@ornl.gov>
Signed-off-by: Daniel Bluhm <bluhmdj@ornl.gov>
Map java.util.Date to TypeScript Date

Signed-off-by: Daniel Bluhm <bluhmdj@ornl.gov>
Signed-off-by: Daniel Bluhm <bluhmdj@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
Signed-off-by: Michael Walsh <walshmm@ornl.gov>
walshmm/feature/ice-annotation-extraction-service

Conflicts:
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/processors/DataElementProcessor.java
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/processors/DefaultFields.java
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/processors/Field.java
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/processors/ImplementationWriter.java
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/processors/InterfaceWriter.java
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/processors/PersistenceHandlerWriter.java
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations/src/main/java/org/eclipse/ice/dev/annotations/processors/VelocityProperties.java
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations/src/test/java/org/eclipse/ice/tests/dev/annotations/processors/DataElementAnnotationTestHelper.java
	org.eclipse.ice.dev/org.eclipse.ice.dev.annotations/src/test/java/org/eclipse/ice/tests/dev/annotations/processors/DataElementProcessorTest.java
	org.eclipse.ice.dev/org.eclipse.ice.dev.pojofromjson/src/main/java/org/eclipse/ice/dev/pojofromjson/PojoFromJson.java

Signed-off-by: Michael Walsh <walshmm@ornl.gov>
@dbluhm
Copy link
Author

dbluhm commented Oct 13, 2020

  1. Sounds good 🙂
  2. Sounds good again
  3. I'll add a README -- intended to and promptly forgot.

@walshmm
Copy link

walshmm commented Oct 13, 2020

  1. Just let me know when. 👍

Copy link
Member

@jayjaybillings jayjaybillings left a comment

Choose a reason for hiding this comment

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

Looks good. This is nice work. Minor changes requested. Also, what's the level of test coverage?

Comment on lines 27 to 31
* Factory for WriterGenerators. Create method parameters represent dependencies
* of a set of generators.
* @author Daniel Bluhm
*/
public class WriterGeneratorFactory {
Copy link
Member

Choose a reason for hiding this comment

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

This is a really nice class. However, it is somewhat complex because of it's abstraction and would really benefit from a little extra documentation, an example, or a unit test. Just a thought!

According to PR feedback. This will improve debugging.
Note: Sonar Lint is not fond of this practice but better experience
while debugging is favorable to this particular Sonar Lint rule.

Signed-off-by: Daniel Bluhm <bluhmdj@ornl.gov>
Signed-off-by: Daniel Bluhm <bluhmdj@ornl.gov>
And move common methods for AnnotationExtractors to it.

Signed-off-by: Daniel Bluhm <bluhmdj@ornl.gov>
And add tests for it.

Signed-off-by: Daniel Bluhm <bluhmdj@ornl.gov>
And other minor fixes.

Signed-off-by: Daniel Bluhm <bluhmdj@ornl.gov>
@dbluhm
Copy link
Author

dbluhm commented Oct 13, 2020

Looks good. This is nice work. Minor changes requested. Also, what's the level of test coverage?

With lombok influencing my coverage it's hard to say exactly what percentage but after removing lombok builders from the coverage (so there would still be equals, hashcode, nonnull, etc.) we're around 90%

Signed-off-by: Daniel Bluhm <bluhmdj@ornl.gov>
Signed-off-by: Daniel Bluhm <bluhmdj@ornl.gov>
Copy link
Member

@jayjaybillings jayjaybillings left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. This looks excellent. Approved from a code perspective. I'll check my other notes in the ticket.

@jayjaybillings
Copy link
Member

  • Sounds good 🙂

  • Sounds good again

  • I'll add a README -- intended to and promptly forgot.

How's the README going? The code looks good and I'm ready to merge. Should we still meet? If so, will @walshmm be joining us?

@dbluhm
Copy link
Author

dbluhm commented Oct 22, 2020

How's the README going? The code looks good and I'm ready to merge. Should we still meet? If so, will @walshmm be joining us?

README is coming along, I've ended up putting more effort into it than I anticipated it would need and even had some interesting questions raised from the exercise of writing a demo processor with all the bits and pieces. From a code perspective, I think this is feature complete. The questions I raised for myself are future work that would build off of what is already present so I think those are best suited for separate PR(s).

@dbluhm
Copy link
Author

dbluhm commented Oct 22, 2020

And I'm still good to meet and discuss changes whenever!

@jayjaybillings
Copy link
Member

Okay, let's catch up in Mattermost about picking a date to talk.

@jayjaybillings
Copy link
Member

Nevermind - I don't think we need to meet to discuss it. I think you addressed all of my questions already. As soon as you push the README, I'll merge it (or you can). Nice work.

Copy link
Member

@jayjaybillings jayjaybillings left a comment

Choose a reason for hiding this comment

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

Approved. Good work.

@jayjaybillings
Copy link
Member

Two things:

  1. Can you fix the conflict?
  2. Can you fix your history?

Otherwise, ready to merge.

@dbluhm dbluhm force-pushed the feature/aes branch 4 times, most recently from def44e4 to eb426d0 Compare October 23, 2020 17:22
With walkthrough of extending annotation processing.

Signed-off-by: Daniel Bluhm <bluhmdj@ornl.gov>
@dbluhm
Copy link
Author

dbluhm commented Oct 23, 2020

After a mess of force pushes, I think we're just going to have to wait for the ECA bot to update after I had to fiddle around with my email associated with my Eclipse Foundation account for EclipseCon. If I had to guess, it seems that it must update on a daily cron job or something as after a few hours it still hasn't updated. I'll check back in tomorrow and ask the bot to revalidate and hopefully everything will be sorted then.

Signed-off-by: Daniel Bluhm <bluhmdj@ornl.gov>
@dbluhm
Copy link
Author

dbluhm commented Oct 23, 2020

Oh, look at that! It decided to update finally. I'll go ahead and merge 🙂

@dbluhm dbluhm merged commit fa78615 into eclipse:next Oct 23, 2020
@dbluhm dbluhm deleted the feature/aes branch October 23, 2020 17:33
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.

None yet

3 participants