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

Add publish script. #23

Merged
merged 4 commits into from
Nov 17, 2017
Merged

Add publish script. #23

merged 4 commits into from
Nov 17, 2017

Conversation

amandeepg
Copy link
Contributor

Using the bintray-release, along with their instructions for publishing to Maven Central

@amandeepg amandeepg requested a review from a team November 16, 2017 04:02
@amandeepg amandeepg requested a review from a team November 16, 2017 07:45
Copy link
Contributor

@fmborghino fmborghino left a comment

Choose a reason for hiding this comment

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

No biggie but we're doing some doc in rst and some in markdown.

build.gradle Outdated
@@ -21,8 +21,8 @@ buildscript {
jcenter()
}
dependencies {
// NOTE: Do not place your application dependencies here; they belong
// in the individual module build.gradle files
classpath 'com.android.tools.build:gradle:2.3.1'
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this still required for a Java only project?

Copy link
Contributor Author

@amandeepg amandeepg Nov 16, 2017

Choose a reason for hiding this comment

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

Good question, file an issue or send a PR for this to discuss separately, I'm not sure.
Edit: Oops, didn't realize I added this. Let me look into it for this PR then.

project {
name "Serial"
artifactId "serial"
packaging project.name == 'compiler' ? 'jar' : 'aar'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can just use packaging jar.

Copy link
Contributor Author

@amandeepg amandeepg Nov 16, 2017

Choose a reason for hiding this comment

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

Based on our POM file here, https://repo1.maven.org/maven2/com/twitter/serial/serial/0.1.5/serial-0.1.5.pom, I think it's actually "packaging 'aar'" that we want, or maybe that's incorrect? Agree that we don't need this ternary though

Copy link
Contributor

Choose a reason for hiding this comment

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

I think JAR is the correct value since the artifact is a jar file.

}
}

publish {
Copy link
Contributor

Choose a reason for hiding this comment

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

All these values can be moved to gradle.properties

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah okay, I wasn't sure what should go there, so I just put stuff I expected would change, i.e. just the version name

repoName = 'Serial'
groupId = 'com.twitter.serial'
artifactId = 'serial'
publishVersion = project.VERSION_NAME
Copy link
Contributor

Choose a reason for hiding this comment

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

project.VERSION_NAME -> VERSION_NAME

@efrohnhoefer
Copy link
Contributor

LGTM 🚢

@amandeepg amandeepg merged commit 200febb into master Nov 17, 2017
@amandeepg amandeepg deleted the publish2 branch November 17, 2017 17:00
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.

4 participants