#2232 Add the possibility to require a certain flag to allow merging a PR
Closed by pingou. Opened by pingou.
force_flag  into  master

Download 2232.patch
no initial comment

You can set the return_value directly inside the patch decorator. I find it more readable. Example here

sure, can do :)

rebased

Maybe if self._enforced_flags is not None:, as I believe we are checking for that here.

maybe rename repo_flags by project_flags since we get the flags from the project class. The usage of repo/project is sometimes a bit confusing

Let's put this in the decorator too ;)

Same here, it could be put in the decorator

Doc string need to be updated s/is missing/has an invalid/

Need to be updated

This is comment is wrong

Same here the comment needs an update

Doc string to be updated

Let's move this to the decorator

Comment need some update

Name of the class needs to be changed and docstring too

I propose the following error message Token %s not found ! This token is not associated with this project

I am not sure this test needs to be repeated, it is the same as in test_update_enforced_flags_invalid_token

same here not sure this test is needed again

The tests comments and docstrings need to be cleaned up a little, but other than that and a few little comment it looks good.

I'm not happy with this change, so I'm going to close it without merging

Pull-Request has been closed by pingou

Metadata