Closed Bug 1283649 Opened 4 years ago Closed 4 years ago

When an attachment is a github pull request link, the pull request diff should be displayed in the edit page

Categories

(bugzilla.mozilla.org :: General, defect, P3)

Production
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: dkl, Assigned: dkl)

References

Details

Attachments

(3 files, 1 obsolete file)

For example the github pull request https://github.com/bugzilla/bugzilla/pull/7
allows you to create a custom URL to see the diff of the pull request at
https://patch-diff.githubusercontent.com/raw/bugzilla/bugzilla/pull/7.diff

We should be able to load the contents of that link from the 'Details' page in the normal patch iframe. Then comments can be added, attributes changed, etc. to the bug report and attachment.

dkl
Attached patch 1283649_1.patch (obsolete) — Splinter Review
Attachment #8768549 - Flags: review?(dylan)
Comment on attachment 8768549 [details] [diff] [review]
1283649_1.patch

Review of attachment 8768549 [details] [diff] [review]:
-----------------------------------------------------------------

I *want* this feature! it's great! Two issues and a nit:

on the patch view function, there is text overflow (screenshot #1).
on the attachment list in the modal UI, "Review" is listed twice. One goes to Github,
the other to Splinter. We should be able to tell those apart.

also the method name makes me cringe.

::: extensions/BMO/Extension.pm
@@ +80,4 @@
>      *Bugzilla::Attachment::bounty_details           = \&_attachment_bounty_details;
>      *Bugzilla::Attachment::external_redirect        = \&_attachment_external_redirect;
>      *Bugzilla::Attachment::can_review               = \&_attachment_can_review;
> +    *Bugzilla::Attachment::get_ghpr_diff            = \&_attachment_get_ghpr_diff;

I hate this name. ghpr--
fetch_github_pr_diff, lets me know you're going to be doing some sort of action to obtain it.
Attachment #8768549 - Flags: review?(dylan) → review-
Attached patch 1283649_2.patchSplinter Review
Attachment #8768549 - Attachment is obsolete: true
Attachment #8768798 - Flags: review?(dylan)
Comment on attachment 8768798 [details] [diff] [review]
1283649_2.patch

r=dylan

this patch is awesome
Attachment #8768798 - Flags: review?(dylan) → review+
fix on commit: make sure splinter is not something search engines will hit in robots.txt
Flags: needinfo?(dkl)
Made github fetches only work for authenticated users.

To https://github.com/mozilla-bteam/bmo.git
   b823202..dc519b0  master -> master

dkl
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Flags: needinfo?(dkl)
Resolution: --- → FIXED
Blocks: 1287895
You need to log in before you can comment on or make changes to this bug.