Skip to content

Commit 3fe4d28

Browse files
committed
Adjust RFC rendered linkifier.
This changes the URL used in the RFC "Rendered" link to link directly to the author's branch instead of a specific commit. The problem with specific commits is that they need to be updated on every push. The existing code didn't handle that (it ignored if it already had a link), and I think SHA blob links are less desirable since they have a risk that reviewers will be looking at outdated content. This also drops the check for `Synchronize` events, since this isn't updating the rendered link on every push.
1 parent d08ab16 commit 3fe4d28

File tree

2 files changed

+21
-4
lines changed

2 files changed

+21
-4
lines changed

src/github.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,10 +284,10 @@ pub struct Issue {
284284

285285
/// The base commit for a PR (the branch of the destination repo).
286286
#[serde(default)]
287-
base: Option<CommitBase>,
287+
pub base: Option<CommitBase>,
288288
/// The head commit for a PR (the branch from the source repo).
289289
#[serde(default)]
290-
head: Option<CommitBase>,
290+
pub head: Option<CommitBase>,
291291
}
292292

293293
/// Contains only the parts of `Issue` that are needed for turning the issue title into a Zulip
@@ -904,6 +904,9 @@ struct PullRequestEventFields {}
904904
#[derive(Clone, Debug, serde::Deserialize)]
905905
pub struct CommitBase {
906906
sha: String,
907+
#[serde(rename = "ref")]
908+
pub git_ref: String,
909+
pub repo: Repository,
907910
}
908911

909912
pub fn files_changed(diff: &str) -> Vec<&str> {

src/handlers/rfc_helper.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,29 @@ pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> {
2323
}
2424

2525
async fn add_rendered_link(ctx: &Context, e: &IssuesEvent) -> anyhow::Result<()> {
26-
if e.action == IssuesAction::Opened || e.action == IssuesAction::Synchronize {
26+
if e.action == IssuesAction::Opened {
2727
let files = e.issue.files(&ctx.github).await?;
2828

2929
if let Some(file) = files.iter().find(|f| f.filename.starts_with("text/")) {
3030
if !e.issue.body.contains("[Rendered]") {
31+
// This URL should be stable while the PR is open, even if the
32+
// user pushes new commits.
33+
//
34+
// It will go away if the user deletes their branch, or if
35+
// they reset it (such as if they created a PR from master).
36+
// That should usually only happen after the PR is closed.
37+
// During the closing process, the closer should update the
38+
// Rendered link to the new location (which we should
39+
// automate!).
40+
let head = e.issue.head.as_ref().unwrap();
41+
let url = format!(
42+
"https://github.com/{}/blob/{}/{}",
43+
head.repo.full_name, head.git_ref, file.filename
44+
);
3145
e.issue
3246
.edit_body(
3347
&ctx.github,
34-
&format!("{}\n\n[Rendered]({})", e.issue.body, file.blob_url),
48+
&format!("{}\n\n[Rendered]({})", e.issue.body, url),
3549
)
3650
.await?;
3751
}

0 commit comments

Comments
 (0)