Fix same link in different file error #26
Conversation
|
Linter failure seems to be due to goreportcard.com being down. |
|
You should be able to retry @saswatamcode since you are maintainer now! |
|
Hm, do you know why this error was created in the first place? (: |
|
I think this error was created to improve performance and avoid duplicate requests in colly. But here it seems to cause this issue...is there a better way to handle this other than using Also, goreportcard seems to still be down so check would fail if I rerun... |
Signed-off-by: Saswata Mukherjee <[email protected]>
saswatamcode
force-pushed
the
same-link-fix
branch
from
f2c2f7f to
42fd909
Compare
| v.c.OnRequest(func(request *colly.Request) { | ||
| v.rMu.Lock() | ||
| defer v.rMu.Unlock() | ||
| request.Ctx.Put("originalURL", request.URL.String()) |
There was a problem hiding this comment.
Nice!
Suggestion: Handcrafted strings like this can be prone to errors. Use constant to reuse such key. Also it does not need to be string to be key. It's recommended to use empty struct (less mem allocated) e.g
var originalURLKey = struct{}{}
There was a problem hiding this comment.
The Get and Put methods seem to only take in strings as highlighted here. Should originalURLKey be passed in using fmt.Sprintf here? But that maybe wouldn’t help with memory as that would be the same as using a string…
There was a problem hiding this comment.
Ah right, this is something library specific not Go context.. ok, let's do constant at least
Signed-off-by: Saswata Mukherjee <[email protected]>
Currently, if there are same links but in different files (come under same file glob passed to mdox) like here and here, mdox throws the error below,
Even though the link is valid.
This PR fixes this and adds a test case for the same.