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.

Sign up for GitHub

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

Human friendly incantations + Auto Closing/Postponing RFCs + More labels #197

Merged
merged 7 commits into from
Apr 30, 2018
Merged

Human friendly incantations + Auto Closing/Postponing RFCs + More labels #197

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a330887
improve rfcbot command parsing to be more human friendly.
Centril Apr 20, 2018
e347795
refactor + auto-close/postpone FFCPed RFCs + better FFCP comment + mo…
Centril Apr 20, 2018
6295b0e
update toolchain := 2018-04-19 + cargo update
Centril Apr 20, 2018
544d05e
refactor consts into a Label enum.
Centril Apr 20, 2018
2a96c07
add/remove_label methods on Issue instead of free functions.
Centril Apr 21, 2018
dac6c55
fix the GH.close_issue command.
Centril Apr 21, 2018
13c38fd
FCP + concern => PFCP per aturon's request; also refactor into execut…
Centril Apr 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 43 additions & 21 deletions src/github/nag.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,30 +458,33 @@ fn evaluate_nags() -> DashResult<()> {
}
};

// Execute FFCP actions:
if is_rfc_repo(&issue) {
match disp {
FcpDisposition::Merge => {
// TODO: This one will require a lot of work to
// auto-merge RFCs and create the tracking issue.
},
FcpDisposition::Close => {
let _ = issue.add_label(Label::Closed);
issue.remove_label(Label::DispositionClose);
issue.close();
},
FcpDisposition::Postpone => {
let _ = issue.add_label(Label::Postponed);
issue.remove_label(Label::DispositionPostpone);
issue.close();
},
}
}
execute_ffcp_actions(&issue, disp);
}

Ok(())
}

fn execute_ffcp_actions(issue: &Issue, disposition: FcpDisposition) {
if !is_rfc_repo(&issue) { return; }

match disposition {
FcpDisposition::Merge => {
// TODO: This one will require a lot of work to
// auto-merge RFCs and create the tracking issue.
},
FcpDisposition::Close => {
let _ = issue.add_label(Label::Closed);
issue.remove_label(Label::DispositionClose);
issue.close();
},
FcpDisposition::Postpone => {
let _ = issue.add_label(Label::Postponed);
issue.remove_label(Label::DispositionPostpone);
issue.close();
},
}
}

fn list_review_requests(proposal_id: i32) -> DashResult<Vec<(GitHubUser, FcpReviewRequest)>> {
use domain::schema::{fcp_review_request, githubuser};

Expand Down Expand Up @@ -846,9 +849,10 @@ impl<'a> RfcBotCommand<'a> {
}
RfcBotCommand::NewConcern(concern_name) => {

if let Some(proposal) = existing_proposal {
if let Some(mut proposal) = existing_proposal {
// check for existing concern
use domain::schema::fcp_concern::dsl::*;
use domain::schema::fcp_proposal::dsl::*;

let existing_concern = fcp_concern
.filter(fk_proposal.eq(proposal.id))
Expand All @@ -870,8 +874,26 @@ impl<'a> RfcBotCommand<'a> {
diesel::insert(&new_concern)
.into(fcp_concern)
.execute(conn)?;

// Take us out of FCP and back into PFCP if need be:
if proposal.fcp_start.is_some() {
// Update DB: FCP is not started anymore.
proposal.fcp_start = None;
match diesel::update(fcp_proposal.find(proposal.id))
Copy link
Member

Choose a reason for hiding this comment

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

Do you think the bot should comment when it's doing this? Could be confusing to do this without any visible status update?

cc @aturon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thinking is that:

let _ = issue.add_label(Label::PFCP);
issue.remove_label(Label::FCP);

serves as visible status update that the bot reacted to the concern :)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think that suffices, given that the concern registration is already a comment.

.set(&proposal)
.execute(conn) {
Ok(_) => (),
Err(why) => {
error!("Unable to mark FCP {} as unstarted: {:?}", proposal.id, why);
return Ok(());
}
}

// Update labels:
let _ = issue.add_label(Label::PFCP);
issue.remove_label(Label::FCP);
}
}

}
}
RfcBotCommand::ResolveConcern(concern_name) => {
Expand Down