#2206 Add the opportunity to enforce a PR-based workflow
Merged by pingou. Opened by pingou.
enforce_pr  into  master

Download 2206.patch

With this feature we are simply dropping all access to the main repo,
thus forcing all contributors to use a PR-based workflow.

In a world where we have CI on PR, this may be an interesting to turn on
and in dist-git this may be even more important.

s/grand/grant/

It seems weird to disallow forks from using the pr_only feature. What if a fork has forks, and the "middle fork" wants to enforce PRs only?

Might want to drop this line.

s/project/app/

Might want to drop this line too.

You might want to move this into a tearDown() method, because this line won't happen if the test fails. If that happened, this could cause other tests to fail in mysterious ways and it might not be completely obvious that the others failed because this line didn't execute.

Alternatively, I think I would recommend using mock to set this in the first place, rather than doing it this way. With mock you wouldn't need a tearDown() and you could still avoid the issue I described here.

I recommend doing this with mock since it is important that it be undone when the test is over.

You could consider adding a config to make the repo have pr_only set to False to make sure the global setting properly overrides the individual repo setting (which seems to be the intention based on the code).

By "config" I mean a line here in the test. Sorry for the bad wording ☺

If the disallowing fork thing is important or you want to keep it, I recommend adding a test case to ensure that behavior works as expected.

LGTM!

I see what you mean, my issue is that currently if the main project uses pr_only, then all its fork will be forced into pr_only as well (resulting in no-one being able to commit on their repo and fork)

I tend to keep a few of these in these tests as they are most helpful to debug when a test fails :)

No problem to add the test, any suggestion on how to change the global config w/ mock?

Found it :)

rebased

1 new commit added

  • Rework the logic for when to drop access to a repo

Reworked the logic in such a way that forks are still allowing commit access if the instance enforces the PR only workflow. Unless the project itself asks for PR only.

@bowlofeggs if you have some time, let me know what you think of this now :)

LGTM!

rebased

Pull-Request has been merged by pingou

Metadata