Amber: Automate Amber Repo Releases

Created on 1 Apr 2018  路  7Comments  路  Source: amberframework/amber

This is a suggestion for us to automate releases as much as possible. I believe as the project grows it would be helpful for us to be able to focus on CODE QUALITY, PERFORMANCE and FEATURES without worrying about releases as much as we are now. This will allow us to focus on reviewing pull requests more thoroughly, and enforce of standards for community contributions while at the same time removing the burden of working and synchronizing project releases.

DRAFT FOR REVIEW

Writing Commit Messages

[patch | minor | major] Changes title

Issue: [link to issue]

Description of the changes, describe why are the changes 
presented are necessary

- List of changes 
- List of changes 
- List of changes 

### What can we expect after these changes

Describe in a small paragraphs the behavior changes that 
are introduced by the commit. This will make it easier for 
future developers to understand not only what has change
but how and why.

Tooling: Travis CI Github Releases

  1. https://docs.travis-ci.com/user/deployment/releases/
  2. https://github.com/crystal-lang-tools/scry/blob/master/.travis.yml

Use already existing scripts for releases

  1. https://github.com/elorest/CRelease
  2. Write custom sh scripts to parse the commit message to determine next version, the script should perform the following

    1. Determine next version from commit

    2. Write commit for next version

    3. Create next version tag/commit. If major release it creates a prerelease tag

    4. Push release tag

  3. Second Script to update Homebrew and AUR packages

Suggestions

  • All PRs requires 2 review minimum
  • Only 1 designated person can merge to master (This person is responsible to merge only and must ensure that the commit message follows the above structure and is descriptive enough since the commit will part of the release notes)
  • Since we would like weekly releases we can merge everything thats open in a weekend and schedule the release or we can allow for Patches and Minor versions to be automatically released.
  • Major changes AKA Breaking changes will not be automatically released, this will require manual input
  • Major AKA Breaking changes will be released as *prerelease (and someone should press the button) and must be tested in production and pass a list of QA *

    • TODO We will need to write a list of QA

  • Only 1 commit per PR all commits must be squashed to a single commit after approval.

We should have automated releases available to all Amber Framework repositories

Alternatives

We can have the version number be bump for each PR, so a PR will have 2 commits, 1 with the actual work perform and a second commit bumping the version number

enhancement Important discussion help-wanted

Most helpful comment

Maybe we're not disagreeing as much as I initially thought.

The situation I don't want to see is:

  1. New contributor sees a help wanted sign on an issue and decides to solve it
  2. Writes code, makes commits, and submits a PR
  3. First reviewer says "Please rewrite your commit history to match this format"

All 7 comments

Can we just set the repo settings to only allow for squash pr merges?

@jwaldrip Occasionally it's necessary to merge without squashing. For instance if someone submits a PR but a core member needs to fix something before merge. It's nice to be able to see both of those commits instead of just the last committer getting credit for all the work. This happened with a PR from bcardiff over the holidays making amber work with 0.24.1. He submitted and then became unresponsive for a couple necessary changes.

Another reason is that during releases we need to show 2 commits or else stable is off from master.

  • Versions are changed on release branch.
  • Changes are pushed up to tags and stable branch.
  • Cli shard.yml amber version is then changed back to master and merged into master branch with PR
  • If the PR is squashed then stable will have a commit that doesn't exist in master despite being in the past.

@amberframework/core-team In the past we've had the rules that largely have been followed.

  • Every PR must have 2 reviews except for in the case of an urgent bug fix that doesn't result in a major change. i.e. fixing a typo or version number.
  • Once a PR has 2 reviews and has been tested both with formal tests and manual tests in a real app it can be merged by (usually) the creator (if core member) unless someone requests a change.

I'm mostly fine with the rest of the suggestions.

I'd also like to propose that if a PR will touch more than 10 files we discuss it in detail in an issue before submitting a PR.

Codifying commit messages and branch flow is going to be a difficult proposal to keep up. I see what you're trying to do, but I don't think it's going to make the projects around here friendly for contributors because it creates an additional burden on a contributor to modify their own workflow to fall in step. As @elorest has pointed out, there are always going to be exceptions.

I do not support the idea of cutting releases automatically, regardless of the contents of the release. What if someone forgets to tag a pull request as [breaking] 20min before the automatic release trigger and it goes out with a patch bump before it can be stopped? It's simply too risky.

This is going to be something that grows and builds over time as requirements change and are added to each release. As a first step, I'd like to suggest simply keeping track of a raw list of items required to make a release happen.

Once the steps are tested and understood, perhaps then a script can be written which allows automatic releases to take place via script, CI, or private website. Until then, documenting the _manual_ steps required to create a release allows _everyone_ to discuss the steps, innovate and simplify, and move forward.

Codifying commit messages and branch flow is going to be a difficult proposal to keep up. I see what you're trying to do, but I don't think it's going to make the projects around here friendly for contributors because it creates an additional burden on a contributor to modify their own workflow to fall in step

I can definitely see that happening, maybe being to explicit in the commit message is not viable at the moment. But is very discouraging when you see very vague commit messages, and trying to understand changes in a PR. Maybe adding the tags [patch | minor | major] in the commit is a little too much but well documented and structured commit messages should be a requirement. People who is not used to will find it cumbersome but trust it would be soon appreciate, specially when you do a git log

Not to say that the community is really big, but I do not see a lot of burden when making contributions to follow these steps. This makes the person contributing to be very thoughtful. One thing I didn't note is to require specs (good test coverage) for each pull requests.

What if someone forgets to tag a pull request as [breaking] 20min before the automatic release trigger and it goes out with a patch bump before it can be stopped?

Nothing gets deployed. We can automate a lot of this. Quite frankly as developers, releases should be something natural and easy to do in the workflow.

...documenting the manual steps required to create a release allows everyone to discuss the steps, innovate and simplify, and move forward.

Completely agree right now thats more of a black box. But I definitely agree that documenting would help to make improvements

Maybe we're not disagreeing as much as I initially thought.

The situation I don't want to see is:

  1. New contributor sees a help wanted sign on an issue and decides to solve it
  2. Writes code, makes commits, and submits a PR
  3. First reviewer says "Please rewrite your commit history to match this format"

Closing this issue. I think the current process is sufficient for now. We can revisit this if we find something is not working.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

faustinoaq picture faustinoaq  路  4Comments

bigforcegun picture bigforcegun  路  3Comments

jaysneg picture jaysneg  路  5Comments

faustinoaq picture faustinoaq  路  5Comments

drujensen picture drujensen  路  6Comments