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 an option to disable the "recheck" part of the comment #158

Merged
merged 5 commits into from
Sep 23, 2024

Conversation

kingthorin
Copy link
Contributor

@kingthorin kingthorin commented Aug 13, 2024

We've found that due to the suggestion in the action's comment that users' comment "recheck" we've ended up with everyone commenting their agreement immediately followed by commenting recheck. Or, commenting both in the same comment. Therefore we'd like the option to disable the "recheck" suggestion.

Note: I have very little experience with GitHub action creation/contribution, if I've edited something I shouldn't have then please let me know, or conversely missed editing something I should have.

Prepared via: dev.github.com
Modeled mainly based on lock-pullrequest-aftermerge handling.

@kingthorin
Copy link
Contributor Author

Hmmm I have no idea why so much of dist/index.js is showing changes. Perhaps dev.github.com adjusted line endings?

@kingthorin
Copy link
Contributor Author

That seems to be what it is. If I choose “Hide white space” when viewing the diff then it appears as expected. I’ll look at pulling it into a real editor and fixing it.

@kingthorin kingthorin force-pushed the recheck-param branch 2 times, most recently from 11afbd2 to 8ec614c Compare August 14, 2024 11:47
@kingthorin
Copy link
Contributor Author

kingthorin commented Aug 14, 2024

I don't seem to be able to revert whatever those white space changes were. The only changes I actually implemented were on line < 1100

When viewing the diff if you select hide white space or hide white space changes then it'll be focused on intended changes.

@kingthorin
Copy link
Contributor Author

Screenshot of diff ignoring whitespace (Click the triangle/control to expand)

image

@kingthorin
Copy link
Contributor Author

Hey crew, I understand you all have day jobs etc and that this isn't priority one for you. It's been a number of weeks, any chance I could get eyes on this? 😀

@kingthorin
Copy link
Contributor Author

@ibakshay sorry to be pushy, I know ppl are busy with life. But, this would really help us and hopefully others too.

@ibakshay
Copy link
Member

@kingthorin, I apologise for a LATE response. i was on vacation during August and then life happened.
I really appreciate your PR! I tested in my test repository and I found out that the fix doesn't work as expected. I am getting an error - link to the test workflow run.

Tip:
You have to add an if check in these two places.

  1. text += '<sub>You can retrigger this bot by commenting **recheck** in this Pull Request. Posted by the ****DCO Assistant Lite bot****.</sub>'
  2. text += '<sub>You can retrigger this bot by commenting **recheck** in this Pull Request. Posted by the **CLA Assistant Lite bot**.</sub>'

@kingthorin
Copy link
Contributor Author

Thanks I'll get those adjusted too!

@kingthorin
Copy link
Contributor Author

Hopefully that's it 😀

@ibakshay
Copy link
Member

Hopefully that's it 😀

You should also add the input to this action.yml file :)
https://github.com/contributor-assistant/github-action/blob/master/action.yml

@kingthorin
Copy link
Contributor Author

No problem, thanks for your help

@kingthorin
Copy link
Contributor Author

Addressed.

@ibakshay
Copy link
Member

ibakshay commented Sep 20, 2024

Addressed.

thanks a lot. Could you please do npm run buildand then push again?
You will see some changes in dist/index.js after you did npm run build. @kingthorin

@kingthorin
Copy link
Contributor Author

Yup I'll have to setup a Linux image, I was doing this all via GitHub.dev

@kingthorin
Copy link
Contributor Author

I was able to do it in a CodeSpace 😀 Hopefully that looks as you expected?

@ibakshay
Copy link
Member

I was able to do it in a CodeSpace 😀 Hopefully that looks as you expected?

It works great. Thanks so much for your contribution! I really appreciate it very much 🚀

Copy link
Member

@ibakshay ibakshay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@ibakshay ibakshay merged commit d755a68 into contributor-assistant:master Sep 23, 2024
4 checks passed
@kingthorin kingthorin deleted the recheck-param branch September 23, 2024 08:19
@kingthorin
Copy link
Contributor Author

Thanks for helping move this along. Sadly a colleague pointed out this may have broken things because the bot “signature” is left off but used as a condition elsewhere.

if (getUseDcoFlag() === 'true') {
return response.data.find(comment => comment.body.match(/.*DCO Assistant Lite bot.*/m))
} else if (getUseDcoFlag() === 'false') {
return response.data.find(comment => comment.body.match(/.*CLA Assistant Lite bot.*/m))

I’ll get a fix PR in today.

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.

2 participants