Repository navigation
Setting Up the Release Process - #3
Conversation
hiranya911
commented
Apr 14, 2017
- Configure the release process using Maven and Nexus plugins.
- Remove the firebase.it.url system property used in the integration tests. The database URL is now inferred from the project ID value in the provided certificate file.
- Reading SDK version from a resource file that is automatically updated by Maven at build time.
* Reformatted all source files * Separated unit and integration tests * Added maven checkstyle plugin * Got the basic build and unit tests working * Scrubbed the codebase for private keys, certificates etc.
…min-java into hkj-release-process
mikelehen
left a comment
There was a problem hiding this comment.
Basically LGTM, but I don't know maven stuff at all so if you want a sanity-check there you'll need to find somebody else. Maybe ask depoll@ to look or recommend a reviewer.
| @@ -0,0 +1,130 @@ | |||
| # Contributing | Firebase Admin Python SDK | |||
There was a problem hiding this comment.
Python? :-P
(same thing in a few places; please search this file for "python" and fix)
| command to invoke the integration test suite: | ||
|
|
||
| ``` | ||
| mvn verify -Dfirebase.it.certificate=path/to/your/serviceAccount.json |
There was a problem hiding this comment.
Why "certificate" instead of "serviceAccount" or something? I'm not really sure if "certificate" is appropriate.
|
|
||
| Make sure to specify the correct path to your downloaded service account key file as the | ||
| `firebase.it.certificate` system property. This command will invoke both unit and integration test | ||
| suites. To execute only the integration tests, run the command as follows: |
There was a problem hiding this comment.
I don't feel strongly, but why would you only want to run the integration tests? It seems more useful to run only the unit tests (since they're presumably faster and would work even without an internet connection). But I think I'd be inclined to just direct folks to run everything (and remove this section).
…tps://github.com/google/guava/wiki/Release21); Excluding the transitive dependency on Guava 17 from Google API client to prevent having 2 versions of Guava in the classpath
depoll
left a comment
There was a problem hiding this comment.
This basically LGTM. It's still probably worth finding someone (e.g. vikrum@) to review the maven bits, but we probably don't need to block on that.
| <dependency> | ||
| <groupId>com.google.guava</groupId> | ||
| <artifactId>guava</artifactId> | ||
| <version>21.0</version> |
There was a problem hiding this comment.
Did we step back to an older version of Guava?
There was a problem hiding this comment.
Yes. As it turns out 21.0 only works on Java 8. Their documentation recommends using 20.0 for Java 7 compatibility: https://github.com/google/guava/wiki/Release21
|
I will merge this PR as it is now. I've tested the Maven stuff by running the release plugin in dryRun mode, and also by pushing a snapshot release to Nexus. There might be other minor glitches, which I will encounter during the next release. I can fix them at that time, and also update our release process docs accordingly. |
|
I imagine we might need to iterate on the POM once we attempt to stage a release (semi-expected/normal given interacting with their system.) |