search

ropensci-org / buffy

Editorial bots generator
https://buffy-ropensci.readthedocs.io/en/latest/
MIT License
1 stars 2 forks source link

Multiple `approve` commands #20

Closed mpadge closed 3 years ago

mpadge commented 3 years ago

@xuanxu There is currently a single approve command:

https://github.com/ropensci-org/buffy/blob/208f2f11467dd86c814d824f7d41cf4846b336ca/config/settings-production.yml#L32-L43

New system will retain this command for submission-type = standard, but will also feature three additional "grades" triggered by commands of:

approve bronze
approve silver
approve gold

I don't know how strict the command matching of command: approve is? Will it be possible to simply add these additional basic_command versions with command: approve bronze, and so on? Or does there need to be some modification of command syntax parsing? Either way, please help!

And to make matters a bit more complicated / refined: It would also be good to ensure that approve bronze/gold/silver commands were not recognised when submission-type = standard, and that simple approve without grade was not recognised when submission-type = stats (and maybe that doing so generates additional message that approve has to have bronze/silver/gold for those submissions).

xuanxu commented 3 years ago

Will it be possible to simply add these additional basic_command versions with command: approve bronze, and so on?

Yes, additional basic_command responders can be added with command: approve gold, command: approve silver and command: approve bronze. They will not conflict wih each other or with the original command: approveas all the command strings are different.

xuanxu commented 3 years ago

It would also be good to ensure that approve bronze/gold/silver commands were not recognised when submission-type = standard, and that simple approve without grade was not recognised when submission-type = stats (and maybe that doing so generates additional message that approve has to have bronze/silver/gold for those submissions).

It looks like to add all this logic the only solution is custom responders

xuanxu commented 3 years ago

I think this extension of the approve command conflicts with this other one: https://github.com/ropensci-org/buffy/issues/8 The resulting command would be confusing, difficult to remember and prone to errors: @helper-bot approve repo-package-name silver

I suggest we keep the command as @helper-bot approve and have the grade information in the body of the issue: Adding a <--grade--><--end-grade-> HTML comment to the submissions template would allow for this flow:

Something similar could be done for the package-name to keep the approve command as easy to use as posible. @mpadge WDYT? cc/ @maelle

maelle commented 3 years ago

It sounds good as I don't think we can do without the repo-package-name for now (when submissions happen via a form it'll be different but for now we can't expect to always be able to read it from the submission).

I suggest "mint" as a shorter command @helper-bot mint silver.

Looking forward to hearing @mpadge's opinion.

mpadge commented 3 years ago

I entirely agree here - putting <---grade---> as a HTML variable is definitely better, and it is really important to make this as easy as possible for editors, which that definitely does. As does @helper-bot mint gold - that is indeed :1st_place_medal: !!

@xuanxu Shall we put the <---grade---> variable in the issue template, or will it only be added by a mint command? If the former, let me know and I'll update straight away.

maelle commented 3 years ago

To me (I know I wasn't asked :innocent: ) having it added via the command sounds safer for now as we can't guarantee issue body integrity at submission.

mpadge commented 3 years ago

Oh yeah, that's a really good point @maelle, thanks!

xuanxu commented 3 years ago

I'd add the HTML comment to the template. We can have a command that looks for the <!--grade--><!--end-grade--> placeholder in the body and if it's not present it add it at the end of the text, so it is "backwards compatible".

mpadge commented 3 years ago

@xuanxu The PR above adds a HTML statsgrade variable to the template. We then just need to modify the approved command to add some if logic:

https://github.com/ropensci-org/buffy/blob/cac6c7cd050c89d42e62aa8caf78fced7c0376fc/config/settings-production.yml#L34-L45

Something like:

if: 
  value_matches:
    statsgrade: bronze
  add_labels:
    6/approved-bronze
if:
  value_matches:
    statsgrade: silver
  add_labels:
    6/approved-silver
if:
  value_matches:
    statsgrade: gold
  add_labels:
    6/approved-gold

How would we now best do that?

xuanxu commented 3 years ago

@mpadge Great! I'll add that to the new responder 👍