Support private registries (non-auth) and fix git resolver #791
Conversation
| let registry = removeSuffix(String(this.registries.yarn.getOption('registry')), '/'); | ||
|
|
||
| if (this.config.registry) { | ||
| registry = this.config.registry; |
There was a problem hiding this comment.
Do you need to remove a possible trailing slash in this case as well? (see removeSuffix above)
There was a problem hiding this comment.
+1 for trailing slash. Could also do the conditional at assignment and keep registry as a const
| registry: 'bower', | ||
| }, await createConfig()); | ||
| await fetcher.fetch(); | ||
| const name = (await fs.readJson(path.join(dir, 'bower.json'))).name; |
There was a problem hiding this comment.
If syntax is supported you could do this:
const { name } = await fs.readJson(...);
palmerj3
force-pushed
the
privateRegAndGit
branch
3 times, most recently
from
59b2471 to
91824da
Compare
|
@palmerj3 looks good once tests are passing. Not by a computer until Sunday so won't be able to pull down and try it out until then. |
|
I tested this code with private registries and it works well. |
|
Can you please rebase |
palmerj3
force-pushed
the
privateRegAndGit
branch
from
91824da to
3c86ae7
Compare
|
@kittens ok all set |
Change-Id: I979d9d43fc4b49de4cd7fba3a14183a0796937c5
palmerj3
force-pushed
the
privateRegAndGit
branch
from
3c86ae7 to
9c0387c
Compare
| const registry = removeSuffix(String(this.registries.yarn.getOption('registry')), '/'); | ||
| const registry = this.config.registry ? | ||
| removeSuffix(this.config.registry, '/') : | ||
| removeSuffix(String(this.registries.yarn.getOption('registry')), '/'); |
There was a problem hiding this comment.
This seems redundant, why not just this.config.registry || this.registries.yarn.getOption('registry) instead of duplicating each of them in the removeSuffix call?
I want to use Yarn at work and this lets me do that. We utilize a private registry and github enterprise.
Test plan
There are currently no tests for npm-registry to begin with so I didn't add anything there.
For git+https I added a test to package-resolver and fetchers.js.