Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(639)

Issue 6745051: code review 6745051: go.talks/present: display twitter names in @name format (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 5 months ago by adg
Modified:
11 years, 5 months ago
Reviewers:
CC:
golang-dev, francesc, r
Visibility:
Public.

Description

go.talks/present: display twitter names in @name format

Patch Set 1 #

Total comments: 2

Patch Set 2 : diff -r 1f15ca58c373 https://code.google.com/p/go.talks #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -0 lines) Patch
M present/parse.go View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 6
adg
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go.talks
11 years, 5 months ago (2012-10-22 04:39:37 UTC) #1
adg
Ping On 22 Oct 2012 15:39, <adg@golang.org> wrote: > Reviewers: golang-dev_googlegroups.com, > > Message: > ...
11 years, 5 months ago (2012-10-24 23:02:27 UTC) #2
francesc
https://codereview.appspot.com/6745051/diff/1/present/parse.go File present/parse.go (right): https://codereview.appspot.com/6745051/diff/1/present/parse.go#newcode370 present/parse.go:370: func parseURL(text string) Elem { Shouldn't this method be ...
11 years, 5 months ago (2012-10-24 23:12:58 UTC) #3
r
LGTM
11 years, 5 months ago (2012-10-24 23:41:57 UTC) #4
adg
*** Submitted as http://code.google.com/p/go/source/detail?r=51956b13eca8&repo=talks *** go.talks/present: display twitter names in @name format R=golang-dev, campoy, r ...
11 years, 5 months ago (2012-10-25 00:00:34 UTC) #5
adg
11 years, 5 months ago (2012-10-25 00:02:09 UTC) #6
https://codereview.appspot.com/6745051/diff/1/present/parse.go
File present/parse.go (right):

https://codereview.appspot.com/6745051/diff/1/present/parse.go#newcode370
present/parse.go:370: func parseURL(text string) Elem {
On 2012/10/24 23:12:58, campoy1 wrote:
> Shouldn't this method be part of link.go and return Link instead of Elem?
> 
> I think it's weird we're explicitly using an extension from the main code.

Probably.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b