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

Update dependencies and mark as isolating for APT #113

Merged
merged 8 commits into from
Nov 17, 2018

Conversation

ZacSweers
Copy link
Collaborator

@ZacSweers ZacSweers commented Nov 17, 2018

Resolves a few things:

Resolves #112
Resolves #106

@ZacSweers ZacSweers self-assigned this Nov 17, 2018
Copy link
Owner

@rharter rharter left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks!

@ZacSweers ZacSweers merged commit aa14903 into master Nov 17, 2018
@ZacSweers ZacSweers deleted the z/updateDependencies branch November 17, 2018 19:33
@tbroyer
Copy link

tbroyer commented Nov 17, 2018

Extension doesn't implement the new API to be marked as isolating.

Also, have you tested incrementality of the processor? It doesn't look like it declares originating elements for the generated sources, which is at least required for isolating processors (not sure about aggregating ones).

@ZacSweers
Copy link
Collaborator Author

that's... really strange o_O. I had it locally for sure, probably got lost in the bouncing around of repos.

For the processor - I was under the impression that aggregating processors didn't have to declare originating elements (since they could be from many places). I'll read up on the docs again

@ZacSweers
Copy link
Collaborator Author

I suppose that's implied here - https://docs.gradle.org/current/userguide/java_plugin.html

It just says isolating must have one, but doesn't specify anything explicitly for aggregating ones. Do you know if we also need to declare the annotated factory interface itself as an originating element? (Assuming yes)

Also - would extensions such as @Memoized imply that its extension should be aggregating since it depends on annotations outside of what's fetched in roundenvironment?

@ZacSweers
Copy link
Collaborator Author

I've added the missing bits in #114 if you want to review

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.

3 participants