Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upIssue 329 #333
Conversation
xd009642
added some commits
Oct 3, 2018
pietroalbini
requested changes
Oct 3, 2018
|
You need to run rustfmt (and clippy) stable to get the CI to pass. Also, I'd personally add the link in another line, along with an URL to the queue, like so: Preview |
| @@ -37,14 +37,14 @@ pub fn run(host: &str, data: &Data, issue: &Issue, args: RunArgs) -> Result<()> | |||
| Message::new() | |||
| .line( | |||
| "ok_hand", | |||
| format!("Experiment **`[{}]({})`** created and queued.", name, host), | |||
| format!("Experiment [**`{}`**](http://{}/ex/{0}) created and queued.", name, host), | |||
This comment has been minimized.
This comment has been minimized.
pietroalbini
Oct 3, 2018
Member
Hmm, I would use https by default here. It's what Crater uses in prod, and ngrok also provides https urls.
src/server/routes/webhooks/mod.rs
Outdated
| let host = headers | ||
| .get("Host") | ||
| .and_then(|h| h.to_str().ok()) | ||
| .ok_or("mission header Host\n")?; |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
xd009642
Oct 3, 2018
Author
Contributor
so the \n is present in the previous lines error strings so I felt it was best to be consistent. If I remove it should I remove the others?
This comment has been minimized.
This comment has been minimized.
xd009642
added some commits
Oct 3, 2018
pietroalbini
approved these changes
Oct 3, 2018
pietroalbini
merged commit 29d2aeb
into
rust-lang-nursery:master
Oct 3, 2018
This comment has been minimized.
This comment has been minimized.
|
Thanks! |
This comment has been minimized.
This comment has been minimized.
|
Changes deployed to production and announced in the changelog |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
xd009642 commentedOct 3, 2018
This pr closes #329 by adding a URL to the report into the create and run comment. It does this using the Host header in the HTTP request and then passes it down to commands::run as an additional argument.
Tested on with my own
craterbot-jrand a test repo. You can see the correct link created here though that link will stop working when I the ngrok url runs out!