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

Automate build and publish with travis-ci.org #62

Open
Rtzq0 opened this issue Jan 2, 2017 · 10 comments
Open

Automate build and publish with travis-ci.org #62

Rtzq0 opened this issue Jan 2, 2017 · 10 comments

Comments

@Rtzq0
Copy link
Collaborator

Rtzq0 commented Jan 2, 2017

This follows on from #23 to track CI integration separately since the original cause of that issue has now been closed.

Goals:

  • Publish to pypi based on tags??
  • Autotest all the things (incl PRs?)
@lottspot
Copy link
Collaborator

lottspot commented Jan 3, 2017

Yeah I think publishing to pypi based on tags makes perfect sense. IMO, we really only need to run tests against PRs to master as long as we make sure that all changes to master come in via PRs from personal forks or topic branches.

@tomkukral
Copy link

@lottspot running tests on travis is simple & inexpensive ... I'd recommend to run test on master as well.

@lottspot
Copy link
Collaborator

lottspot commented Jan 6, 2017

@tomkukral Testing PRs destined for master and then testing master after merging them is simply redundant. There's just no point.

@Rtzq0
Copy link
Collaborator Author

Rtzq0 commented Jan 6, 2017

In the general case this is true and I agree that this is redundant, however AFAIK there is no way from preventing direct pushes to master for people who have the authority to merge PRs (which means that we're on the 'honor system').

Therefore I think testing of master should be a "nice to have if easy" if it doesn't pose a big problem.

@lottspot
Copy link
Collaborator

lottspot commented Jan 6, 2017

It's trivial to disallow pushes to the master branch by enabling required pull request reviews

@lottspot
Copy link
Collaborator

lottspot commented Jan 6, 2017

We would need @madduck to approve and enable such a change to master branch policy though, if that's something we want to do

@Rtzq0
Copy link
Collaborator Author

Rtzq0 commented Jan 6, 2017

Unless they changed that feature while I wasn't looking, it only requires reviews on PRs but does not block direct pushes. The only SCM I know that does this is Phabricator. Happy to be shown wrong though.

Not disagreeing that in 99% of cases it's redundant though.

@lottspot
Copy link
Collaborator

lottspot commented Jan 6, 2017

It does in fact block direct pushes

/tmp
14:50 $ git clone [email protected]:lottspot/protected-branch-repo.git
Cloning into 'protected-branch-repo'...
remote: Counting objects: 3, done.
remote: Total 3 (delta 0), reused 0 (delta 0), pack-reused 0
Receiving objects: 100% (3/3), done.
/tmp
14:51 $ cd protected-branch-repo/
/tmp/protected-branch-repo [master|✔]
14:51 $ echo '' > README.md
14:51 $ git commit -am 'Truncate README'
[master c610429] Truncate README
 1 file changed, 1 insertion(+), 1 deletion(-)
/tmp/protected-branch-repo [master ↑·1|✔]
14:51 $ git push origin master
Counting objects: 3, done.
Writing objects: 100% (3/3), 250 bytes | 0 bytes/s, done.
Total 3 (delta 0), reused 0 (delta 0)
remote: error: GH006: Protected branch update failed for refs/heads/master.
remote: error: At least one approved review is required
To github.com:lottspot/protected-branch-repo.git
 ! [remote rejected] master -> master (protected branch hook declined)
error: failed to push some refs to '[email protected]:lottspot/protected-branch-repo.git'

I've added you as a collaborator to that repository if you'd like to experiment for yourself

@Rtzq0
Copy link
Collaborator Author

Rtzq0 commented Jan 6, 2017

WHOOOO! that is some serious good news. It did not used to do that.

Recommend if @madduck is okay with it that we turn this on.

@lottspot
Copy link
Collaborator

lottspot commented Jan 6, 2017

As a note to @madduck -- by default that feature allows an administrator override so that you as the owner can still push/merge freely to the master branch

AndrewPickford pushed a commit to AndrewPickford/reclass that referenced this issue Sep 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants