bors argument parsing does not report errors
I recently issued the command @bors r+ rollup+, because I have seen plenty of cases where one has to add a + after a command to make it happen. Not so for rollup, it seems, but bors also did not complain. Can we get error messages when there is a word+ or word- but bors does not actually recognize this as a command?
Oh, and as a bonus (but less important), accepting rollup+ as well as rollup would be nice :)
I think it's better just let homu recognize rollup+, try+ and treeclosed+.
This is a good first issue for somebody new to Homu.
A little guidance for somebody taking this task:
- [ ] Create a test similar to https://github.com/rust-lang/homu/blob/abd0083ba6a738e0bc1a1bc69e64676ff9032ebf/homu/tests/test_parse_issue_comment.py#L257-L269 that sets the
bodyvariable to a comment you might expect – one that includes a@bors retry+command. - [ ] Observe that the test fails when you run
python3 setup.py test. - [ ] Update https://github.com/rust-lang/homu/blob/abd0083ba6a738e0bc1a1bc69e64676ff9032ebf/homu/parse_issue_comment.py#L139 to pass the test.
- [ ] Identity other similar commands that would benefit from the
{commandname}+syntax and make the same changes for those commands.
and
treeclosed+.
@kennytm What does this even mean if there's no priority?
It would mean treeclosed=1
@kennytm I would always like that to scream loudly that I've done something wrong because >= 1 is entirely ineffective.
I have no mental model as to why some things have a + suffix and some don't.
I mean, treeclose- is a thing, right? So then not having treeclose+ seems really weird?
@RalfJung treeclosed- means that you remove the previous treeclosed=P. If you use rollup- it means removing the rollup annotation; but treeclosed+ meaning treeclosed=1 is "a bug" from a UX perspective of the person doing the treeclosing. If treeclosed+ was equivalent to treeclosed=500 I would be fine with it.
So treeclosed=0 is not "un-closing" the tree? That is surprising, too.
I don't know; I've never tried it :)