-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Publish android artifacts with publish to s3 gradle plugin #33441
Publish android artifacts with publish to s3 gradle plugin #33441
Conversation
@@ -1,7 +1,7 @@ | |||
buildscript { | |||
ext { | |||
gradlePluginVersion = '4.0.2' | |||
kotlinVersion = '1.3.61' | |||
kotlinVersion = '1.5.20' |
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.
Previous kotlin version was resulting in some build failures, but unfortunately I can't remember the details of the failure. The upgrade didn't cause any issues, so I don't think we need to be concerned about this version upgrade.
@@ -12,10 +12,16 @@ buildscript { | |||
wordpressUtilsVersion = '1.22' | |||
espressoVersion = '3.0.1' | |||
aztecVersion = 'v1.3.45' | |||
willPublishReactNativeAztecBinary = properties["publishReactNativeAztecVersion"]?.trim() as boolean | |||
willPublishReactNativeAztecBinary = properties["willPublishReactNativeAztecBinary"]?.toBoolean() ?: false |
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 am still not particularly happy about this naming (and the similar one in react-native-bridge
), but I can't think of a better name that communicates what it's about.
} | ||
} | ||
|
||
apply plugin: 'com.android.library' | ||
apply plugin: 'kotlin-android' | ||
apply plugin: 'com.github.dcendents.android-maven' | ||
apply plugin: 'maven-publish' |
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.
maven-publish
plugin will be applied by publish-to-s3
gradle plugin.
secretKey project.hasProperty('awsSecretKey') ? project.properties['awsSecretKey'] : System.getenv('AWS_SECRET_KEY') | ||
} | ||
} | ||
addDependenciesToPom(pom) |
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'd like to remove this and the artifact bundleReleaseAar
property and instead utilize from.release
(which should add the necessary dependencies), but my initial test failed and I think it deserves its own PR, so I am leaving it as is.
mavenPublishGroupId 'org.wordpress-mobile.gutenberg-mobile' | ||
mavenPublishArtifactId 'react-native-aztec' |
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.
These properties are used to check if a version is already published to S3. It should match the information in the publish
block.
I am going to re-visit this later and try to get rid of it - or at least extract the values so we don't have to reference them twice.
@@ -1,53 +0,0 @@ | |||
#!/bin/bash |
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.
This script is no longer necessary because:
- We are moving the logic the
gutenberg-mobile
repository - Some of the logic is covered by
publish-to-s3
plugin - We are simplifying this process to address some developer frustrations
include ':react-native-prompt-android' | ||
project(':react-native-prompt-android').projectDir = new File(rootProject.projectDir, '../../../node_modules/react-native-prompt-android/android') | ||
|
||
if (hasProperty("willPublishReactNativeBridgeBinary")) { |
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.
This if/else
block is necessary to be able to configure the project when the node_modules
folder is missing from the project. With Buildkite integration, we are separating the Android artifact publishing task from the Android resource bundling, so node_modules
will no longer be available and the project is expected to only work with the index.android.bundle
file.
Oguz and I went over this together yesterday and it LGTM! |
28e21ea
to
10d0d3b
Compare
10d0d3b
to
9cfb723
Compare
Size Change: 0 B Total Size: 1.07 MB ℹ️ View Unchanged
|
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 this @oguzkocer! Also thanks for all the explanatory comments on this PR! 😄
* Remove maven plugin from react-native-bridge * Relax the react-native-bridge android project setup requirements * Adds publish-to-s3 plugin to react-native-bridge Android * Integrate publish-to-s3 plugin to react-native-aztec * Bring back the manual pom generation * Fix property name for checking node_modules in react-native-bridge android * Update kotlin to 1.5.20 in Android projects * Update react-native-bridge artifact id to react-native-gutenberg-bridge * Remove publish-aztec-and-bridge.sh * Update publish-to-s3 plugin to 0.6.1
Description
Update
react-native-bridge
andreact-native-aztec
projects to use publish-to-s3-gradle-plugin. This plugin allows us to simplify the logic in the clients by adding aprepareToPublishToS3
Gradle task which will set the version name according to the provided parameters and verify that version is not yet published.Please note that this PR should not be merged before gutenberg-mobile PR (which I'll create and link here soon ™️ ) is approved and ready to be merged as this PR contains breaking changes. At this point, I am only looking for code review and will ask for it to be tested in the
gutenberg-mobile
PR instead. I'll also add some line comments explaining the reasoning behind some changes. cc @jkmasselHow has this been tested?
react-native-bridge
&react-native-aztec
libraries and verified that they build in my local environmentgutenberg-mobile
repository and verified that artifacts were published correctlyreact-native-editor
is still yet to be tested although I don't expect it to be impacted by these changes.Types of changes
Build file changes for
react-native-bridge
&react-native-aztec
Android libraries.Checklist:
*.native.js
files for terms that need renaming or removal).