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

Add Option to not post failure status to github when there is no successful policy #711

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

RoryDoherty
Copy link
Contributor

@RoryDoherty RoryDoherty commented Feb 8, 2024

This should address the following issue:

#709

@palantirtech
Copy link
Member

Thanks for your interest in palantir/policy-bot, @RoryDoherty! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@RoryDoherty
Copy link
Contributor Author

I also think the settings in config could be used to implement this: #387
Could have another option called approve_pr_on_success_status

@bluekeyes
Copy link
Member

Thanks for your contribution! Could you please recreate this as two PRs, one that adds the equality operator and one for the status changes? The equality operator seems straightforward and should be easy to merge, while the status behavior is something I'd like to think about more. While the code change is minimal, it adds some new concepts to Policy Bot that deserve consideration.

@RoryDoherty RoryDoherty changed the title Add Option to not post status to github on failures and add equals operator for modifications Add Option to not post failure status to github when there is no successful policy Feb 9, 2024
@RoryDoherty
Copy link
Contributor Author

@bluekeyes thanks for the feedback, I've updated this PR so that it is just the option to not set failure status checks and opened this PR #712 with the changes for adding the equals operator for file modifications

@RoryDoherty
Copy link
Contributor Author

@bluekeyes would you be able to take a look at this PR anytime soon?
I'm eager to also implement #387 as these missing features are blocking us from currently rolling this out
I don't want to start on 387 however until I know your happy with the config settings being in the .policy.yml file

@bluekeyes
Copy link
Member

Sorry about the delay getting back to this. As I mentioned on the linked issue, I think this feature is something we can add with two changes:

  1. Only skip posting a status when there are no matching rules. If there are matching rules, pending and failure status should still be posted.
  2. Make the option to configure this a feature of the server configuration rather than the policy. We use Policy Bot on thousands of repositories and we want to make sure the behavior is consistent, where individual projects cannot opt-in to this behavior. A server-level flag would allow people who don't care about this consistency to get the skipping behavior.

@RoryDoherty
Copy link
Contributor Author

Unfortunately the changes you've suggested would make this feature unusable for us

  1. If a matching policy fails I just want it to be ignored completely, I wouldn't want to post the failure status as it unnecessarily adds failed checks to a commit which would make people think something was wrong with the commit
  2. Would it be possible to have this as a server level flag to allow the setting in the policy file, This way you can have repos mix and match but only if the server has the option enabled? This would save from having to deploy two different instances to get the same feature set

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants