Skip to content
This repository has been archived by the owner on Feb 24, 2021. It is now read-only.

Enable incremental annotation processing #153

Merged
merged 9 commits into from
Jun 30, 2020
Merged

Conversation

hadilq
Copy link
Contributor

@hadilq hadilq commented Jun 12, 2020

This PR is created after arrow-kt/arrow#2032 issue to enable incremental annotation processing. The following is the list of assumptions and changes.

  • Update auto-service to 1.0-rc7 where based on Support incremental annotation processing  google/auto#615 must support incremental annotation processing. We may need to create a PR for https://github.com/arrow-kt/arrow to update the GOOGLE_AUTO_SERVICE_VERSION value,
  • Use File instead of Filer when the packageName contains Java keywords like const or try
  • Ignore double attempt to generate a file where throws FilerException: Attempt to reopen a file for path ...
  • We may need to create PRs for other Arrow projects to add kapt.incremental.apt=true line to gradle.properties files.
  • Additionally, there are some fix of indention, where just happened when I reformat working files in this PR

@aballano aballano requested a review from pakoito June 12, 2020 10:40
compileOnly "com.google.auto.service:auto-service:$GOOGLE_AUTO_SERVICE_VERSION"
kapt "com.google.auto.service:auto-service:$GOOGLE_AUTO_SERVICE_VERSION"
compileOnly "com.google.auto.service:auto-service:1.0-rc7"
kapt "com.google.auto.service:auto-service:1.0-rc7"
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this awesome PR @hadilq 🙌 I was thinking about this kind of changes and I think the mistake was to locate all the versions in a centralized place without filtering them.

I mean, in this case, GOOGLE_AUTO_SERVICE_VERSION (and GOOGLE_COMPILE_TESTING_VERSION as well) is just being used in this repository so I think it doesn't make sense to be placed in arrow repository.

So please, feel free to locate those versions in this repository and remove them from arrow repository.

I think arrow repository should contain versions that are relevant and can impact to all the libraries. What do you think about it?

Thanks again @hadilq !!

Copy link
Contributor Author

@hadilq hadilq Jun 12, 2020

Choose a reason for hiding this comment

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

Thank you @rachelcarmena for this great suggestion. I'm totally agree and you can find the changes in b8d1703 and arrow-kt/arrow#2158

Copy link
Member

Choose a reason for hiding this comment

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

Awesome, thanks @hadilq for your contribution!!

@hadilq
Copy link
Contributor Author

hadilq commented Jun 29, 2020

Hi everyone! It took more than two weeks to review it. Please feel free to give me some feedback. Thanks :)

Copy link
Member

@nomisRev nomisRev left a comment

Choose a reason for hiding this comment

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

Hey @hadilq,

Sorry for the late response. Not many contributors have experience in the old Arrow-Meta implementation, so not many reviewers available.
This looks good to me, and would great to have for a last 0.11.0 release which will still rely on this. Also for working on Arrow that'll be great! So thanks a lot for this contribution :)

cc\ @raulraja could you also give this a look?

Copy link
Member

@raulraja raulraja left a comment

Choose a reason for hiding this comment

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

Thanks @hadilq ! As you may now soon we will retire kapt for meta and as Simon pointed out there is not many people in this area to review.

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.

5 participants