Skip to content

Commit

Permalink
Accept multiple @rfcbot invocations per comment (rust-lang#211)
Browse files Browse the repository at this point in the history 
closes rust-lang#168
  • Loading branch information
@Centril
Centril committed Jun 20, 2018
1 parent 8fdc2d8 commit b07f080
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 35 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,5 @@ rfcbot behaves so that the people who develop rfcbot or interact with it can
understand the bot better.

## Changes

+ rfcbot now accepts multiple invocations / commands per comment you post.
7 changes: 7 additions & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ serde_json = "1.0"
toml = "0.4"
url = "1.4"
urlencoded = "0.5"
maplit = "1.0.1"

[dependencies.chrono]
features = ["serde"]
Expand Down
43 changes: 43 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,49 @@ Only the first of these commands will be registered:

Examples are in each section.

### Command grammar

rfcbot accepts roughly the following grammar:

```ebnf
merge ::= "merge" | "merged" | "merging" | "merges" ;
close ::= "close" | "closed" | "closing" | "closes" ;
postpone ::= "postpone" | "postponed" | "postponing" | "postpones" ;
cancel ::= "cancel | "canceled" | "canceling" | "cancels" ;
review ::= "reviewed" | "review" | "reviewing" | "reviews" ;
concern ::= "concern" | "concerned" | "concerning" | "concerns" ;
resolve ::= "resolve" | "resolved" | "resolving" | "resolves" ;
line_remainder ::= .+$ ;
ws_separated ::= ... ;
subcommand ::= merge | close | postpone | cancel | review
| concern line_remainder
| resolve line_remainder
;
invocation ::= "fcp" subcommand
| "pr" subcommand
| "f?" ws_separated
| subcommand
;
grammar ::= "@rfcbot" ":"? invocation ;
```

Multiple occurrences of `grammar` are allowed in each comment you make on GitHub.
This means that the following is OK:

```
@rfcbot merge
Some stuff you want to say...
@rfcbot concern foobar
Explain the concern...
```

### Final Comment Period

Before proposing a final comment period on an issue/PR/RFC, please double check to make sure that the correct team label(s) has been applied to the issue. As of 9/17/16, rfcbot recognizes these labels:
Expand Down
20 changes: 5 additions & 15 deletions src/github/client.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,6 @@ pub const DELAY: u64 = 300;

type ParameterMap = BTreeMap<&'static str, String>;

macro_rules! params {
($($key: expr => $val: expr),*) => {{
let mut map = BTreeMap::<_, _>::new();
$(map.insert($key, $val);)*
map
}};
}

header! { (TZ, "Time-Zone") => [String] }
header! { (Accept, "Accept") => [String] }
header! { (RateLimitRemaining, "X-RateLimit-Remaining") => [u32] }
Expand Down Expand Up @@ -94,7 +86,7 @@ impl Client {

pub fn issues_since(&self, repo: &str, start: DateTime<Utc>) -> DashResult<Vec<IssueFromJson>> {
self.get_models(&format!("{}/repos/{}/issues", BASE_URL, repo),
Some(&params! {
Some(&btreemap! {
"state" => "all".to_string(),
"since" => format!("{:?}", start),
"per_page" => format!("{}", PER_PAGE),
Expand All @@ -107,7 +99,7 @@ impl Client {
start: DateTime<Utc>)
-> DashResult<Vec<CommentFromJson>> {
self.get_models(&format!("{}/repos/{}/issues/comments", BASE_URL, repo),
Some(&params! {
Some(&btreemap! {
"sort" => "created".to_string(),
"direction" => "asc".to_string(),
"since" => format!("{:?}", start),
Expand Down Expand Up @@ -164,7 +156,7 @@ impl Client {

pub fn close_issue(&self, repo: &str, issue_num: i32) -> DashResult<()> {
let url = format!("{}/repos/{}/issues/{}", BASE_URL, repo, issue_num);
let payload = serde_json::to_string(&params!("state" => "closed"))?;
let payload = serde_json::to_string(&btreemap!("state" => "closed"))?;
let mut res = self.patch(&url, &payload)?;

if StatusCode::Ok != res.status {
Expand Down Expand Up @@ -208,9 +200,7 @@ impl Client {
text: &str)
-> DashResult<CommentFromJson> {
let url = format!("{}/repos/{}/issues/{}/comments", BASE_URL, repo, issue_num);

let payload = serde_json::to_string(&params!("body" => text))?;

let payload = serde_json::to_string(&btreemap!("body" => text))?;
// FIXME propagate an error if it's a 404 or other error
self.deserialize(&mut self.post(&url, &payload)?)
}
Expand All @@ -225,7 +215,7 @@ impl Client {
repo,
comment_num);

let payload = serde_json::to_string(&params!("body" => text))?;
let payload = serde_json::to_string(&btreemap!("body" => text))?;

// FIXME propagate an error if it's a 404 or other error
self.deserialize(&mut self.patch(&url, &payload)?)
Expand Down
103 changes: 83 additions & 20 deletions src/github/nag.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,13 @@ pub fn update_nags(comment: &IssueComment) -> DashResult<()> {

let subteam_members = subteam_members(&issue)?;

// attempt to parse a command out of the comment
if let Ok(command) = RfcBotCommand::from_str(&comment.body) {
// Attempt to parse all commands out of the comment
let mut any = false;
for command in RfcBotCommand::from_str_all(&comment.body) {
any = true;

// don't accept bot commands from non-subteam members
// Don't accept bot commands from non-subteam members.
// Early return because we'll just get here again...
if subteam_members.iter().find(|&u| u == &author).is_none() {
info!("command author ({}) doesn't appear in any relevant subteams",
author.login);
Expand All @@ -104,8 +107,9 @@ pub fn update_nags(comment: &IssueComment) -> DashResult<()> {
});

debug!("rfcbot command is processed");
}

} else {
if !any {
ok_or!(resolve_applicable_feedback_requests(&author, &issue, comment),
why => error!("Unable to resolve feedback requests for comment id {}: {:?}",
comment.id, why));
Expand Down Expand Up @@ -578,6 +582,32 @@ fn parse_command_text<'a>(command: &'a str, subcommand: &'a str) -> &'a str {
/// Parses all subcommands under the fcp command.
/// If `fcp_context` is set to false, `@rfcbot <subcommand>`
/// was passed and not `@rfcbot fcp <subcommand>`.
///
/// @rfcbot accepts roughly the following grammar:
///
/// merge ::= "merge" | "merged" | "merging" | "merges" ;
/// close ::= "close" | "closed" | "closing" | "closes" ;
/// postpone ::= "postpone" | "postponed" | "postponing" | "postpones" ;
/// cancel ::= "cancel | "canceled" | "canceling" | "cancels" ;
/// review ::= "reviewed" | "review" | "reviewing" | "reviews" ;
/// concern ::= "concern" | "concerned" | "concerning" | "concerns" ;
/// resolve ::= "resolve" | "resolved" | "resolving" | "resolves" ;
///
/// line_remainder ::= .+$ ;
/// ws_separated ::= ... ;
///
/// subcommand ::= merge | close | postpone | cancel | review
/// | concern line_remainder
/// | resolve line_remainder
/// ;
///
/// invocation ::= "fcp" subcommand
/// | "pr" subcommand
/// | "f?" ws_separated
/// | subcommand
/// ;
///
/// grammar ::= "@rfcbot" ":"? invocation ;
fn parse_fcp_subcommand<'a>(
command: &'a str,
subcommand: &'a str,
Expand Down Expand Up @@ -876,20 +906,20 @@ impl<'a> RfcBotCommand<'a> {
Ok(())
}

pub fn from_str(command: &'a str) -> DashResult<RfcBotCommand<'a>> {
// get the tokens for the command line (starts with a bot mention)
let command = command
.lines()
.find(|&l| l.starts_with(RFC_BOT_MENTION))
.ok_or(DashError::Misc(None))?
.trim_left_matches(RFC_BOT_MENTION)
.trim_left_matches(':')
.trim();

let mut tokens = command.split_whitespace();
pub fn from_str_all(command: &'a str) -> impl Iterator<Item = RfcBotCommand<'a>> {
// Get the tokens for each command line (starts with a bot mention)
command.lines()
.filter(|&l| l.starts_with(RFC_BOT_MENTION))
.map(Self::from_invocation_line)
.filter_map(Result::ok)
}

fn from_invocation_line(command: &'a str) -> DashResult<RfcBotCommand<'a>> {
let mut tokens = command.trim_left_matches(RFC_BOT_MENTION)
.trim_left_matches(':')
.trim()
.split_whitespace();
let invocation = tokens.next().ok_or(DashError::Misc(None))?;

match invocation {
"fcp" | "pr" => {
let subcommand = tokens.next().ok_or(DashError::Misc(None))?;
Expand All @@ -913,6 +943,7 @@ impl<'a> RfcBotCommand<'a> {
_ => parse_fcp_subcommand(command, invocation, false),
}
}

}

struct RfcBotComment<'a> {
Expand Down Expand Up @@ -1120,6 +1151,34 @@ impl<'a> RfcBotComment<'a> {
mod test {
use super::*;

#[test]
fn multiple_commands() {
let text = r#"
someothertext
@rfcbot: resolved CONCERN_NAME
somemoretext
somemoretext
@rfcbot: fcp cancel
foobar
@rfcbot concern foobar
"#;

let cmd = RfcBotCommand::from_str_all(text).collect::<Vec<_>>();
assert_eq!(cmd, vec![
RfcBotCommand::ResolveConcern("CONCERN_NAME"),
RfcBotCommand::FcpCancel,
RfcBotCommand::NewConcern("foobar"),
]);
}

fn ensure_take_singleton<I: Iterator>(mut iter: I) -> I::Item {
let singleton = iter.next().unwrap();
assert!(iter.next().is_none());
singleton
}

macro_rules! justification {
() => { "\n\nSome justification here." };
}
Expand Down Expand Up @@ -1148,8 +1207,11 @@ somemoretext")
let body = concat!("@rfcbot: ", $cmd);
let body_no_colon = concat!("@rfcbot ", $cmd);

let with_colon = RfcBotCommand::from_str(body).unwrap();
let without_colon = RfcBotCommand::from_str(body_no_colon).unwrap();
let with_colon =
ensure_take_singleton(RfcBotCommand::from_str_all(body));

let without_colon =
ensure_take_singleton(RfcBotCommand::from_str_all(body_no_colon));

assert_eq!(with_colon, without_colon);
assert_eq!(with_colon, expected);
Expand Down Expand Up @@ -1220,8 +1282,9 @@ somemoretext
somemoretext";

let with_colon = RfcBotCommand::from_str(body).unwrap();
let without_colon = RfcBotCommand::from_str(body_no_colon).unwrap();
let with_colon = ensure_take_singleton(RfcBotCommand::from_str_all(body));
let without_colon =
ensure_take_singleton(RfcBotCommand::from_str_all(body_no_colon));

assert_eq!(with_colon, without_colon);
assert_eq!(with_colon, RfcBotCommand::ResolveConcern("CONCERN_NAME"));
Expand Down
2 changes: 2 additions & 0 deletions src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ extern crate serde_json;
extern crate toml;
extern crate url;
extern crate urlencoded;
#[macro_use]
extern crate maplit;

#[macro_use]
mod macros;
Expand Down

0 comments on commit b07f080

Please sign in to comment.