-
-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adding ref capturing #42
Conversation
49ee453
to
1b71330
Compare
lib/index.js
Outdated
|
||
urlInfo.ref = ""; | ||
urlInfo.filepath = ""; | ||
if ((splits.length > 3) & (["blob", "tree"].indexOf(splits[2]) != -1)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of indexOf(...) != -1
you can do: ["blob", "tree"].includes(splits[2])
.
Also, I think &
is a typo (unless you really meant it), I assume it should be &&
.
Maybe it would be helpful to add the type to the urlInfo (tree
or blob
).
😁
// Find a better property name maybe
urlInfo.type = splits[2]
Nice work overall! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 to comments, I think they're all addressed in latest push (+ a test).
LMKWYT!
friendly ping! anything else I should do on this one? |
Sorry for delay... some unexpected things happened and I my schedule was modified. 🙈 |
Umm, tests are not passing, can you please double-check that? |
ah oops - github said travis passed so I didn't double-check it, lemme see what's up |
Yes, I noticed that too. It may be because of the node version. I didn’t have time to debug.
…Sent from my iPhone
On 21 Jan 2018, at 20:45, Chris Holdgraf ***@***.***> wrote:
ah oops - github said travis passed so I didn't double-check it, lemme see what's up
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
b97a52d
to
bc8da63
Compare
oops, I think I missed a |
all travis errors but one are fixed:
looks like a JS version error, yeah? |
bc8da63
to
3880dd0
Compare
swapped out |
(I think this is ready to go whenever you are @IonicaBizau :-) ) |
This is a first shot at ref / filepath capturing. Still a WIP but is this in the right direction?
to do:
will close #41