-
Notifications
You must be signed in to change notification settings - Fork 84
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 comment-key input #82
Add comment-key input #82
Conversation
@developit any chance this can get feedback sometime? |
@developit Is it possible this PR could get a review? |
Co-authored-by: Bruno Bodian <[email protected]>
@developit Any chance for a review here? |
Closing this PR to clean up my list of open PRs. Please ping me if you'd like me to reopen the PR. |
@JoviDeCroock @rschristian are either of y'all able to review this PR? (Just picking the last couple folks I see reviewing and merging in this repository, hope that's okay) |
Thanks for the ping, I'll definitely try to take a look. Just need to figure out how to test it. |
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.
Looks good to me, though if you could add some documentation to the readme for this new option it'd be much appreciated.
Sorry this sat for so long without movement.
2fc32c1
to
1b56a4a
Compare
1b56a4a
to
79684ef
Compare
All good, thanks for the review. I've pushed changes with docs to the README and resolved the merge conflict. |
Addresses #81 using the suggested implementation.
I'm not totally sure how changes to this are tested, mostly because the CI only runs the action with limited options. Open to recommendations for how to properly test this.