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

Make AEgir friendly to new Github perms set up #225

Closed
daviddias opened this issue May 7, 2018 · 10 comments
Closed

Make AEgir friendly to new Github perms set up #225

daviddias opened this issue May 7, 2018 · 10 comments
Assignees
Labels
P1 High: Likely tackled by core team if no one steps up

Comments

@daviddias
Copy link
Member

Following up ipfs/team-mgmt#600 (comment), we need AEgir to start creating it's release commits on a separate branch. The current situation means we always have to disable github perms to publish.

@daviddias daviddias added the P1 High: Likely tackled by core team if no one steps up label May 7, 2018
@victorb victorb self-assigned this May 7, 2018
@victorb
Copy link
Member

victorb commented May 7, 2018

Branch Protection has been temporary reverted so we can figure out how to make it work with the release flow.

victorb added a commit to ipfs-inactive/ci-sync that referenced this issue May 7, 2018
Ref: ipfs/aegir#225

License: MIT
Signed-off-by: Victor Bjelkholm <[email protected]>
@daviddias
Copy link
Member Author

Lead Maintainers keep running into trouble to do npm releases (because they are not Github Admins). I'm moving this to a P0 as this is blocking a lot of the progress.

@daviddias daviddias added P0 Critical: Tackled by core team ASAP status/ready Ready to be worked and removed P1 High: Likely tackled by core team if no one steps up labels May 30, 2018
@victorb
Copy link
Member

victorb commented May 30, 2018

@diasdavid I'm not sure what's wrong here... https://github.com/ipfs/ci-sync/blob/master/checks/github_branch_protection.go#L56-L68 has the people who should be able to push, required status checks have been removed and required reviews of PRs have been removed.

What's currently missing/what's the error people hit right now?

@alanshaw
Copy link
Member

For the js-ipfs release I was getting:

> aegir release -t node -t browser "--no-lint" "--no-test"

 ✔ Build
 ✔ Update Contributors
 ✔ Bump Version: v0.29.0 -> v0.29.1
 ✔ Generate Changelog
 ✔ Commit to Git
 ⠏ Push to GitHub
   Generate GitHub Release
   Publish documentation
   Publish to npm
remote: error: GH006: Protected branch update failed for refs/heads/master.        
remote: error: Required status check "continuous-integration/jenkins/pr-merge" is expected.        
To github.com:ipfs/js-ipfs.git
 * [new tag]         v0.29.1 -> v0.29.1
 ! [remote rejected] master -> master (protected branch hook declined)
error: failed to push some refs to '[email protected]:ipfs/js-ipfs.git'

@alanshaw
Copy link
Member

For the js-ipfs-api release I was getting:

> aegir release  "--no-lint" "--no-test"

 ✔ Build
 ✔ Update Contributors
 ✔ Bump Version: v22.0.0 -> v22.0.1
 ✔ Generate Changelog
 ✔ Commit to Git
 ⠧ Push to GitHub
   Generate GitHub Release
   Publish documentation
   Publish to npm
remote: error: GH006: Protected branch update failed for refs/heads/master.        
remote: error: At least 1 approving review is required by reviewers with write access.        
To github.com:ipfs/js-ipfs-api.git
 * [new tag]         v22.0.1 -> v22.0.1
 ! [remote rejected] master -> master (protected branch hook declined)
error: failed to push some refs to '[email protected]:ipfs/js-ipfs-api.git'

@victorb
Copy link
Member

victorb commented May 30, 2018

Removing branch protection fully for master for now so we don't get blocked while figuring out the right way of setting it, until we have better release process that works with it.

@victorb
Copy link
Member

victorb commented Oct 3, 2018

I think what we need here is to solve the whole "Automatic publishing of packages" story.

What I think would work would be to instead of aegir pushing directly to GitHub, it creates a PR for the release (also gives us the benefit of getting the option to double-check releases before they happen) which CI can run tests on, and then merged.

However, the npm release happens after the merge, so maybe we need to split the release commands into a prepare-release command that does everything related to files and GitHub while the release command doesn't change anything in SCM, it just does the NPM release and publishes the docs. There is a risk of things not being in sync (between npm released and GitHub released) while in-between commands though...

@ipfs/javascript-team thoughts on solutions for this?

@victorb victorb added P1 High: Likely tackled by core team if no one steps up and removed P0 Critical: Tackled by core team ASAP labels Oct 3, 2018
@vmx
Copy link
Member

vmx commented Oct 4, 2018

The prepare-release and release separation sounds good to me. A future next step could then be that the merge of such a PR does the npm release automatically.

@jacobheun
Copy link
Contributor

I think we could tie this into the release candidate flow that started with the discussion at ipfs/js-ipfs#1533.

This could split the flow up into 2 main stages.

  1. Release Candidate
    1.1. Package version is bumped
    1.2 The build happens
    1.3. A github tag is created from that version
    1.4. An npm release is done for that version and is tagged next instead of latest
    1.5 A PR is generated from the tag, to master

  2. Publishing
    2.1. The PR from 1.5 is merged
    2.2. The published npm version is given the latest tag
    2.3. The docs are published

This would give us a lot of flexibility in testing our releases, and would ensure that the published npm package does not change from what we tested in stage 1, which I am a big fan of.

@hugomrdias
Copy link
Member

feel free to re open this if this still happens.

@ghost ghost removed the status/ready Ready to be worked label Feb 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 High: Likely tackled by core team if no one steps up
Projects
None yet
Development

No branches or pull requests

6 participants