Conversation
This PR: - Accepts mixed-case owner names - Adds a test to that effect - Updates the README to clarify the instructions and correctly state that `blacklisted_labels` is required. - Changes the error message to read more understandably (i.e. the original text had inverted logic in its text).
|
@dkhmelenko : hi there! Thanks for this super-helpful tool. Would you mind reviewing this? I had run into this problem in my own use of the tool. |
|
|
||
| ### blacklisted_labels | ||
|
|
||
| **mandatory** |
There was a problem hiding this comment.
why do you make blacklisted_labels as a mandatory property?
There was a problem hiding this comment.
@dkhmelenko : please correct me if I'm wrong, but I ran into #753 (i.e. my GitHub Action fails if my autoapprove.yml doesn't have the blacklisted_labels field, and succeeds if it does [with an empty array]).
| const lowerLogin = pr.user.login.toLowerCase() | ||
| const ownerSatisfied = config.from_owner.length === 0 || config.from_owner.some((owner: string) => owner.toLowerCase() === lowerLogin) |
There was a problem hiding this comment.
That's an interesting case, out of curiosity: when can it happen that we have different cases of the owner? Or do you want to prevent a situation when GitHub username has different case than the one in the config file?
There was a problem hiding this comment.
I'm trying to address the situation where the yml file contains a different case from what GitHub has. Since GitHub handles are case-insensitive, it feels like the yml format should similarly not enforce casing.
This PR:
blacklisted_labelsis required.