-
Notifications
You must be signed in to change notification settings - Fork 696
Migrate to AndroidX (and relevant updates) #72
Conversation
mkano9
commented
Dec 9, 2018
- AndroidX migration
- Updated targetsdk/compilesdk to 28
- Updated gradle wrapper
- Updated kotlin to 1.3.10
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
I'm not familiar with buck, but it seems the buck set up is no longer functional, so I didn't update the buck files. I also read this description. 59cd5ef |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this. I'm unsure if I can merge this in just yet, since some internal dependencies likely wont work with AndroidX, but should soon.
.idea/codeStyleSettings.xml
Outdated
@@ -1,251 +0,0 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't delete the code style settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ops, not intentional. I will revert this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted this. My IDE was automatically removing it.
kotlin : '1.2.41', | ||
supportLibrary: '27.0.2', | ||
buildTools : '28.0.3', | ||
kotlin : '1.3.10', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes to unrelated versions should be split out into a separate pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kotlin 1.2.41 was not compiling, so I had to update beyond the current version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Buchanan
@@ -2,4 +2,4 @@ distributionBase=GRADLE_USER_HOME | |||
distributionPath=wrapper/dists | |||
zipStoreBase=GRADLE_USER_HOME | |||
zipStorePath=wrapper/dists | |||
distributionUrl=https\://services.gradle.org/distributions/gradle-4.5.1-bin.zip | |||
distributionUrl=https\://services.gradle.org/distributions/gradle-4.9-all.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes like this should be split out into a separate pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't remember if it was necessary to update this. I will double-check.
… the latest supported
@@ -2,4 +2,4 @@ distributionBase=GRADLE_USER_HOME | |||
distributionPath=wrapper/dists | |||
zipStoreBase=GRADLE_USER_HOME | |||
zipStorePath=wrapper/dists | |||
distributionUrl=https\://services.gradle.org/distributions/gradle-4.9-all.zip | |||
distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.3-bin.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4.5.1 fails to compile with error message saying that 4.6 is minimum required. I updated to the latest pre-5 since 5.x doesn't work yet.
Hi @xiphirx, it's ready for review. I pushed another commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xiphirx has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.