-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: configure excluded repositories and commands #56
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs to be a bit more powerful, i.e. I don't think anyone would object to reviewers being requested.
It should have multiple levels,
e.g.
- repo
- repo:command
- repo:command1,command2
Along with tests and the code itself shouldn't be in the router it should be in it's own file which can be tested more easily
Another option, could be to use https://github.com/probot/octokit-plugin-config as it provides config at a repository level. Potentially also at org level via This only provides a config at deployment of application. |
Looks interesting does that mean it loads single files from github? I get that doesn't use API limits. Not currently using probot here FWIW. |
Yes it can load a single yml config file and parse it to a object with sensible defaults. See release-drafter for usage: https://github.com/release-drafter/release-drafter/blob/master/lib/config.js This is only partly related to probot. This is an octokit plugin. So since your using octokit, you can use it. |
That's why I went with an env var, easy to configure at the org/app level, to avoid fetching a file every time.
Totally, will fixes my other fr. |
@lemeurherve problem I can add jenkinsci deployment of github-comment-ops to any github org. |
I might miss something but if people want to use this app in their org they can deploy the chart to setup their instance as they please? |
To be able to configure their own exclusions? 🤷 |
With environment variables they cannot. With a config file in a repository they could. |
I think a config file would give us the flexibility we need, org level defaults and then allow repository overrides, something like: config:
commands:
- name: transfer
permission: member
enabled: true
- name: labels
permission: member
allowed_labels: ['enhancement', 'chore', 'bug']
enabled: true
- name: close
enabled: false
- name: reopen
enabled: false |
Yup this would work because configs are deep merged :) An array would be perfectly merged with defaults in the org. |
I've started this approach here: |
Closing in favor of #60 |
Ref: #54
I'm proposing to define excluded repositories with an env var containing the list of excluded repositories separated by a comma.
(PR in draft, need to add the corresponding value to the chart)
To exclude a repo, an help desk issue could be opened to add the repo name to this env var.
WDYT @timja @dduportal @daniel-beck