Support url based evaluation #1
Conversation
| // Generate token using https://docs.github.com/en/github/authenticating-to-github/creating-a-personal-access-token | ||
| npx supported https://github.com/stefanpenner/supported | ||
| npx supported https://github.com/stefanpenner/supported/tree/some-branch | ||
| npx supported https://test.githubprivate.com/stefanpenner/supported -t $TOKEN |
There was a problem hiding this comment.
The GITHUB_TOKEN environment variable is a convention which we can piggy back on more details
I have fine having -t as a way to override, but if a user has $GITHUB_TOKEN already set It would be great if we could seamlessly use that.
What do you think?
There was a problem hiding this comment.
We may also want to provide a link to documentation which helps people generate said token.
There was a problem hiding this comment.
We have documentation links to create TOKEN is in the error messages. https://github.com/stefanpenner/supported/pull/1/files/a4dd4064e13150cba88efa07a1c6eb35d0c7a01b#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R13
I will add it in the readme as well.
There was a problem hiding this comment.
Sorry for the delayed response on this.
I checked to see if I can find GITHUB_TOKEN in my local. I couldn't find this. Do you have it in your local environment?
There was a problem hiding this comment.
Added documentation in the readme file
| hostUrl: flags.hostUrl, | ||
| }); | ||
| // if project root is a URL then use homedir as root | ||
| projectRoot = require('os').homedir(); |
There was a problem hiding this comment.
This is surprising, I understanding reading the npmrc from the the homedir, but what if someone has a spurious lockfile or package.json in there homedir? This happens surprisingly often, and leads to a confusing and potentially tricky problem too debug.
Is there a more resilient alternative?
There was a problem hiding this comment.
We are not using lock file or package.json from the homedir even if it exists. those are all read from in-memory files passed. The only npmrc we read is from homedir to avoid possible settings in the current project. The hope is not to have any custom npmrc in homedir.
| ...localConfig, | ||
| }; | ||
| } catch (e) { | ||
| throw new Error( |
There was a problem hiding this comment.
I'm always nervous about catching all errors, is there a way to detect the expected exception if there was a parse failure and provide a nicer error? (assuming the existing error isn't itself already nice)
There was a problem hiding this comment.
We can append the actual error this error so that we are not hiding actual error.
There was a problem hiding this comment.
In the new change, I have appended the actual error.
I personally didn't see any error. I have just put the try-catch just to put a nice error message before we throw the actual error stack.
Cases covered: 1. Github repo URL 2. Private Github Repo (token needed) 3. Braches in the repo 4. External URL which can return package.json, yarn.lock, npmrc (optional) files Test cases added for each cases All operations done in memory
Support URL as input.
Cases covered:
1. Github repo URL
2. Private Github Repo (token needed)
3. Braches in the repo
4. External URL which can return package.json, yarn.lock, npmrc (optional) files
Test cases added for each case
All operations are done in memory