Skip to content

Commit 6b0729d

Browse files
anpanp
committed
Adding FCP label to issues, updating comment text.
Also updated Misc error to have optional string for logging.
1 parent 2eb16f7 commit 6b0729d

File tree

3 files changed

+85
-27
lines changed

3 files changed

+85
-27
lines changed

‎src/error.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ pub enum DashError {
1919
Serde(serde_json::error::Error),
2020
R2d2Timeout(r2d2::GetTimeout),
2121
DieselError(diesel::result::Error),
22-
Misc,
22+
Misc(Option<String>),
2323
}
2424

2525
impl From<hyper::error::Error> for DashError {
@@ -51,7 +51,7 @@ impl From<DashError> for iron::IronError {
5151
DashError::Serde(e) => Box::new(e),
5252
DashError::R2d2Timeout(e) => Box::new(e),
5353
DashError::DieselError(e) => Box::new(e),
54-
DashError::Misc => {
54+
DashError::Misc(_) => {
5555
Box::new(io::Error::new(io::ErrorKind::Other, "miscellaneous error"))
5656
}
5757
},

‎src/github/client.rs

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use chrono::{DateTime, UTC};
1010
use hyper;
1111
use hyper::client::{RedirectPolicy, RequestBuilder, Response};
1212
use hyper::header::Headers;
13+
use hyper::status::StatusCode;
1314
use serde::Deserialize;
1415
use serde_json;
1516

@@ -66,17 +67,17 @@ impl Client {
6667

6768
let v = match v.as_object() {
6869
Some(v) => v,
69-
None => return Err(DashError::Misc),
70+
None => return Err(DashError::Misc(None)),
7071
};
7172

7273
let repo = match v.get("name") {
7374
Some(n) => {
7475
match n.as_str() {
7576
Some(s) => format!("{}/{}", org, s),
76-
None => return Err(DashError::Misc),
77+
None => return Err(DashError::Misc(None)),
7778
}
7879
}
79-
None => return Err(DashError::Misc),
80+
None => return Err(DashError::Misc(None)),
8081
};
8182

8283
repos.push(repo);
@@ -167,7 +168,7 @@ impl Client {
167168

168169
Ok(try!(serde_json::from_str::<PullRequestFromJson>(&buf)))
169170
} else {
170-
Err(DashError::Misc)
171+
Err(DashError::Misc(None))
171172
}
172173
}
173174

@@ -191,6 +192,23 @@ impl Client {
191192
None
192193
}
193194

195+
pub fn add_label(&self, repo: &str, issue_num: i32, label: &str) -> DashResult<()> {
196+
let url = format!("{}/repos/{}/issues/{}/labels",
197+
BASE_URL, repo, issue_num);
198+
let payload = serde_json::to_string(&[label])?;
199+
200+
let mut res = self.post(&url, &payload)?;
201+
202+
match res.status {
203+
StatusCode::Ok => Ok(()),
204+
_ => {
205+
let mut body = String::new();
206+
res.read_to_string(&mut body)?;
207+
Err(DashError::Misc(Some(body)))
208+
}
209+
}
210+
}
211+
194212
pub fn new_comment(&self,
195213
repo: &str,
196214
issue_num: i32,

‎src/github/nag.rs

Lines changed: 61 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -144,11 +144,29 @@ fn evaluate_nags() -> DashResult<()> {
144144
proposal.fcp_start = Some(UTC::now().naive_utc());
145145
diesel::update(fcp_proposal.find(proposal.id)).set(&proposal).execute(conn)?;
146146

147-
// TODO attempt to add the final-comment-period label
147+
// attempt to add the final-comment-period label
148+
// TODO only add label if FCP > 1 day
149+
let label_res = GH.add_label(&issue.repository, issue.number, "final-comment-period");
150+
151+
let added_label = match label_res {
152+
Ok(()) => true,
153+
Err(why) => {
154+
error!("Unable to add FCP label to {}#{}: {:?}",
155+
&issue.repository,
156+
issue.number,
157+
why);
158+
false
159+
}
160+
};
161+
162+
let comment_type = CommentType::FcpAllReviewedNoConcerns {
163+
added_label: added_label,
164+
author: &initiator,
165+
status_comment_id: proposal.fk_bot_tracking_comment,
166+
};
148167

149168
// leave a comment for FCP start
150-
let fcp_start_comment = RfcBotComment::new(&issue,
151-
CommentType::FcpAllReviewedNoConcerns);
169+
let fcp_start_comment = RfcBotComment::new(&issue, comment_type);
152170
fcp_start_comment.post(None)?;
153171
}
154172
}
@@ -276,8 +294,7 @@ fn cancel_fcp(author: &GitHubUser, issue: &Issue, existing: &FcpProposal) -> Das
276294
diesel::delete(fcp_proposal.filter(id.eq(existing.id))).execute(conn)?;
277295

278296
// leave github comment stating that FCP proposal cancelled
279-
let comment = RfcBotComment::new(issue,
280-
CommentType::FcpProposalCancelled(author));
297+
let comment = RfcBotComment::new(issue, CommentType::FcpProposalCancelled(author));
281298
let _ = comment.post(None);
282299

283300
Ok(())
@@ -314,7 +331,7 @@ impl FcpDisposition {
314331
"merge" => Ok(FcpDisposition::Merge),
315332
"close" => Ok(FcpDisposition::Close),
316333
"postpone" => Ok(FcpDisposition::Postpone),
317-
_ => Err(DashError::Misc),
334+
_ => Err(DashError::Misc(None)),
318335
}
319336
}
320337
}
@@ -536,18 +553,18 @@ impl<'a> RfcBotCommand<'a> {
536553
let command = command.lines()
537554
.filter(|&l| l.starts_with(RFC_BOT_MENTION))
538555
.next()
539-
.ok_or(DashError::Misc)?
556+
.ok_or(DashError::Misc(None))?
540557
.trim_left_matches(RFC_BOT_MENTION)
541558
.trim_left_matches(':')
542559
.trim();
543560

544561
let mut tokens = command.split_whitespace();
545562

546-
let invocation = tokens.next().ok_or(DashError::Misc)?;
563+
let invocation = tokens.next().ok_or(DashError::Misc(None))?;
547564

548565
match invocation {
549566
"fcp" | "pr" => {
550-
let subcommand = tokens.next().ok_or(DashError::Misc)?;
567+
let subcommand = tokens.next().ok_or(DashError::Misc(None))?;
551568

552569
debug!("Parsed command as new FCP proposal");
553570

@@ -558,7 +575,7 @@ impl<'a> RfcBotCommand<'a> {
558575
"cancel" => Ok(RfcBotCommand::FcpCancel),
559576
_ => {
560577
error!("unrecognized subcommand for fcp: {}", subcommand);
561-
Err(DashError::Misc)
578+
Err(DashError::Misc(Some(format!("found bad subcommand: {}", subcommand))))
562579
}
563580
}
564581
}
@@ -582,15 +599,16 @@ impl<'a> RfcBotCommand<'a> {
582599
"reviewed" => Ok(RfcBotCommand::Reviewed),
583600
"f?" => {
584601

585-
let user = tokens.next().ok_or(DashError::Misc)?;
602+
let user = tokens.next()
603+
.ok_or(DashError::Misc(Some("no user specified".to_string())))?;
586604

587605
if user.len() == 0 {
588-
return Err(DashError::Misc);
606+
return Err(DashError::Misc(Some("no user specified".to_string())));
589607
}
590608

591609
Ok(RfcBotCommand::FeedbackRequest(&user[1..]))
592610
}
593-
_ => Err(DashError::Misc),
611+
_ => Err(DashError::Misc(None)),
594612
}
595613
}
596614
}
@@ -607,7 +625,11 @@ enum CommentType<'a> {
607625
&'a [(GitHubUser, FcpReviewRequest)],
608626
&'a [(GitHubUser, FcpConcern)]),
609627
FcpProposalCancelled(&'a GitHubUser),
610-
FcpAllReviewedNoConcerns,
628+
FcpAllReviewedNoConcerns {
629+
author: &'a GitHubUser,
630+
status_comment_id: i32,
631+
added_label: bool,
632+
},
611633
FcpWeekPassed,
612634
}
613635

@@ -625,7 +647,7 @@ impl<'a> RfcBotComment<'a> {
625647

626648
match self.comment_type {
627649
CommentType::FcpProposed(initiator, disposition, reviewers, concerns) => {
628-
let mut msg = String::from("Team member ");
650+
let mut msg = String::from("Team member @");
629651
msg.push_str(&initiator.login);
630652
msg.push_str(" has proposed to ");
631653
msg.push_str(disposition.repr());
@@ -645,7 +667,7 @@ impl<'a> RfcBotComment<'a> {
645667
}
646668

647669
if concerns.is_empty() {
648-
msg.push_str("\nNo concerns currently listed.");
670+
msg.push_str("\nNo concerns currently listed.\n");
649671
} else {
650672
msg.push_str("\nConcerns:\n\n");
651673
}
@@ -678,15 +700,31 @@ impl<'a> RfcBotComment<'a> {
678700

679701
Ok(msg)
680702
}
703+
681704
CommentType::FcpProposalCancelled(initiator) => {
682705
Ok(format!("@{} proposal cancelled.", initiator.login))
683706
}
684-
CommentType::FcpAllReviewedNoConcerns => {
685-
Ok("All relevant subteam members have reviewed. No concerns remain.".to_string())
707+
708+
CommentType::FcpAllReviewedNoConcerns { author, status_comment_id, added_label } => {
709+
let mut msg = String::new();
710+
711+
msg.push_str(":bell: **This is now entering its final comment period**, ");
712+
msg.push_str("as per the [review above](");
713+
self.add_comment_url(&mut msg, status_comment_id);
714+
msg.push_str("). :bell:");
715+
716+
if !added_label {
717+
msg.push_str("\n\n*psst @");
718+
msg.push_str(&author.login);
719+
msg.push_str(", I wasn't able to add the `final-comment-period` label,");
720+
msg.push_str(" please do so.*");
721+
}
722+
723+
Ok(msg)
686724
}
725+
687726
CommentType::FcpWeekPassed => {
688-
// TODO add ping to original proposal author
689-
Ok("It has been one week since all blocks to the FCP were resolved.".to_string())
727+
Ok("The final comment period is now complete.".to_string())
690728
}
691729
}
692730
}
@@ -703,6 +741,8 @@ impl<'a> RfcBotComment<'a> {
703741
fn post(&self, existing_comment: Option<i32>) -> DashResult<CommentFromJson> {
704742
use config::CONFIG;
705743

744+
// TODO don't do this if the issue is closed
745+
706746
if CONFIG.post_comments {
707747

708748
let text = self.format()?;
@@ -716,7 +756,7 @@ impl<'a> RfcBotComment<'a> {
716756
info!("Skipping comment to {}#{}, comment posts are disabled.",
717757
self.repository,
718758
self.issue_num);
719-
Err(DashError::Misc)
759+
Err(DashError::Misc(None))
720760
}
721761
}
722762
}

0 commit comments

Comments
 (0)