alstr / todo-to-issue-action

Action that converts TODO comments to GitHub issues on push.
MIT License
605 stars 115 forks source link

Add support for running on a workflow triggered by a pull request #81

Closed nickderobertis closed 2 years ago

nickderobertis commented 2 years ago

First, thanks for the great action! Very useful.

I was trying to use this action in a workflow that was triggered by pull_request opened, but it was failing because the INPUT_COMMITS was coming in as None, and the main script checks len(client.commits). At that point it was failing to find the length of NoneType.

In the case of a PR event, we do not have the commits attribute, but we do have event.pull_request.diff_url that directly gives the URL of a diff. Further, github.event.before will not be set, but github.base_ref will.

I was able to get this working in a fork by adding a new DIFF_URL input and defaulting it to github.event.pull_request.diff_url, and having it check for the diff_url before checking the length of commits.

I also made BEFORE check github.base_ref if it does not find github.event.before. This should not be strictly necessary as BEFORE doesn't get used when DIFF_URL is used, so I'm happy to remove this change from the PR. But I have confirmed that this allows BEFORE to be set for either PR triggers or commit triggers.

alstr commented 2 years ago

Thanks very much for this! I'll aim to take a look through it soon. Some changes to the diff processing were made in #72 to accommodate initial commits, so we'll just have to reconcile the two different features.

nickderobertis-ch commented 2 years ago

Happy to help, let me know if you want to discuss anything. I think these changes should be compatible already, but I did not thoroughly test it in commit triggers. It is working well for me in my fork so there's no rush on my end. Thanks!

alstr commented 2 years ago

Thanks, this is next on my list to test! 😄