Bug 197790 - Bugzilla should convert "r12345" to a trac.webkit.org link
Summary: Bugzilla should convert "r12345" to a trac.webkit.org link
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
: 168722 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-05-10 11:57 PDT by Jer Noble
Modified: 2019-05-13 16:10 PDT (History)
6 users (show)

See Also:


Attachments
Patch (5.12 KB, patch)
2019-05-10 11:59 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (5.06 KB, patch)
2019-05-13 14:29 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2019-05-10 11:57:38 PDT
Bugzilla should convert "r12345" to a trac.webkit.org link
Comment 1 Jer Noble 2019-05-10 11:59:28 PDT
Created attachment 369575 [details]
Patch
Comment 2 Alexey Proskuryakov 2019-05-12 14:36:42 PDT
*** Bug 168722 has been marked as a duplicate of this bug. ***
Comment 3 Alexey Proskuryakov 2019-05-12 14:53:33 PDT
Comment on attachment 369575 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=369575&action=review

I guess this should work in the overwhelming majority of cases.

> Websites/bugs.webkit.org/extensions/Trac/Extension.pm:32
> +use Bugzilla::Constants;
> +use Bugzilla::Group;
> +use Bugzilla::User;

Which of these are actually used?

> Websites/bugs.webkit.org/extensions/Trac/Extension.pm:38
> +    my ($self, $args) = @_;
> +        my $regexes = $args->{'regexes'};

Too much indentation.

> Websites/bugs.webkit.org/extensions/Trac/Extension.pm:49
> +    return qq{<a href="//trac.webkit.org/$revision">$text</a>};

Why not say "https:"? These are separate servers, and while both are https only, there is no strict coupling.
Comment 4 Jer Noble 2019-05-13 14:27:10 PDT
(In reply to Alexey Proskuryakov from comment #3)
> Comment on attachment 369575 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=369575&action=review
> 
> I guess this should work in the overwhelming majority of cases.

Yep, and we can clean up and add more regex's if we come across more cases.

> > Websites/bugs.webkit.org/extensions/Trac/Extension.pm:32
> > +use Bugzilla::Constants;
> > +use Bugzilla::Group;
> > +use Bugzilla::User;
> 
> Which of these are actually used?

Turns out, none. I've removed them and the extension still works.

> > Websites/bugs.webkit.org/extensions/Trac/Extension.pm:38
> > +    my ($self, $args) = @_;
> > +        my $regexes = $args->{'regexes'};
> 
> Too much indentation.

Fixed.

> > Websites/bugs.webkit.org/extensions/Trac/Extension.pm:49
> > +    return qq{<a href="//trac.webkit.org/$revision">$text</a>};
> 
> Why not say "https:"? These are separate servers, and while both are https
> only, there is no strict coupling.

Sure; I guess we don't have to worry about CORS stuff during actual navigation.
Comment 5 Jer Noble 2019-05-13 14:29:27 PDT
Created attachment 369782 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2019-05-13 16:09:12 PDT
Comment on attachment 369782 [details]
Patch for landing

Clearing flags on attachment: 369782

Committed r245261: <https://trac.webkit.org/changeset/245261>
Comment 7 WebKit Commit Bot 2019-05-13 16:09:13 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2019-05-13 16:10:22 PDT
<rdar://problem/50743739>