Bug 197790 – Bugzilla should convert "r12345" to a trac.webkit.org link

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

Bugzilla should convert "r12345" to a trac.webkit.org link

[Jer Noble]

Created attachment 369575 [details]
Patch

[Alexey Proskuryakov]

*** Bug 168722 has been marked as a duplicate of this bug. ***

[Alexey Proskuryakov]

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.

[Jer Noble]

(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.

[Jer Noble]

Created attachment 369782 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 369782 [details]
Patch for landing

Clearing flags on attachment: 369782

Committed r245261: <https://trac.webkit.org/changeset/245261>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/50743739>