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

Additions for automated deployments to bintray via travis-ci #82

Merged
merged 2 commits into from
Jan 25, 2018
Merged

Additions for automated deployments to bintray via travis-ci #82

merged 2 commits into from
Jan 25, 2018

Conversation

spenserpothier
Copy link
Contributor

No description provided.

Copy link
Contributor

@jkinkead jkinkead left a comment

Choose a reason for hiding this comment

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

Suggestion: If we update the target branch for release from master to something else (like, say, releases), we can integrate a code review process into the release process.

This DOES mean we'd need to allow merge commits to handle these. Thoughts?

.travis.yml Outdated
language: java
install: true
script:
- ./gradlew clean check generatePomFileForXrpcMavenPublication publishToMavenLocal
Copy link
Contributor

Choose a reason for hiding this comment

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

I would make this clean check test (unless check subsumes test), and put the artifact-generation tasks in before_deploy.

Copy link
Contributor

Choose a reason for hiding this comment

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

check does subsume test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved the publish tasks to before_deploy

@andyday
Copy link
Contributor

andyday commented Jan 25, 2018

@jkinkead it is wise to avoid a specialized release branch if possible. any shared branches add to the complexity of the lifecycle and merge process. i would recommend doing a release on every successful merge to master. this requires each PR to include CHANGELOG and set the new version if needed. the version could also be auto incremented based on a Major, Minor, Patch indicator set by the PR creator... this would also require automatic Github Release using API... I wouldn't recommend this approach for larger projects like kubernetes or the like, but for a smaller library like this it makes sense to me at least.

@jkinkead
Copy link
Contributor

Right now, fully-automated builds aren't happening (it's been hard enough to get travis building). It might be a good next-step, but I'd like to keep the discussion in the context of this current change.

The issue here is that the current approach requires that, for a release to happen, the person running a release needs to have push permissions to master (needs to be a project admin). This isn't great. Having a shared release branch would fix this problem: Any person with normal write permissions could trigger a release.

It DOES add complexity, since it requires something like:

  • configure Nordstrom/xrpc as upstream
  • git fetch upstream
  • git branch -D release
  • git checkout upstream/master
  • git checkout -b release
  • git push release upstream
  • ./gradlew release

Instead of:

  • configure Nordstrom/xrpc as upstream
  • git fetch upstream
  • git checkout master
  • ./gradlew release

The question is: Is the added complexity above worth the ability for non-maintainers to deploy?

* Move publish tasks to `before_deploy` section of travis-ci configuration.
@spenserpothier
Copy link
Contributor Author

I think at this point the added complexity adds marginal value. I've added #83 to discuss the release branches, but personally I don't think this should be a blocker for this PR. As this PR currently stands, we at least get all future PRs built and tests run against them and just the version updates need to be done by a github Nordstrom/xrpc maintainer, but the rest is handled by travisci.
Also worth noting, Travis also includes a github releases deployer that can be added.

@jkinkead
Copy link
Contributor

SGTM. Mostly I'd like for occasional contributors to be empowered to get their patches out without having to bother an admin.

@spenserpothier spenserpothier merged commit 0c54fd6 into Nordstrom:master Jan 25, 2018
@spenserpothier spenserpothier deleted the add-travisci-deployments branch January 25, 2018 20:43
@andyday
Copy link
Contributor

andyday commented Jan 25, 2018

@jkinkead given the sequence you've listed above, if scripted and the fact that the release branch is a temporary branch it should do the trick...

pdex pushed a commit to pdex/xrpc-fork that referenced this pull request Jan 29, 2018
…om#82)

* Additions for automated deployments to bintray via travis-ci

* PR Feedback

* Move publish tasks to `before_deploy` section of travis-ci configuration.
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.

3 participants