Add a unit test to require registry.npm.js as the package registry.#613
Add a unit test to require registry.npm.js as the package registry.#613k8s-ci-robot merged 1 commit intokubernetes-client:masterfrom
Conversation
drubin
left a comment
There was a problem hiding this comment.
Minor suggestion, but it looks good. if you make the changes I will approve.
Also thanks for this! Its surprisingly clean
|
Comments addressed. |
src/package_test.ts
Outdated
| const dep = deps[key]; | ||
| const resolved = new URL(dep.resolved); | ||
| expect(resolved.hostname).to.equal('registry.npmjs.org'); | ||
| expect(resolved.protocol ).to.equal('https); |
There was a problem hiding this comment.
| expect(resolved.protocol ).to.equal('https); | |
| expect(resolved.protocol).to.equal('https); |
not sure how this space got in here.
src/package_test.ts
Outdated
|
|
||
| it('package-lock should only reference official public npm registry', () => { | ||
| const deps = require('../package-lock.json').dependencies; | ||
| for (const key in deps) { |
There was a problem hiding this comment.
It's not enough to loop through the top level - each of dependencies can have its own dependencies (e.g.
Line 38 in d18e69d
|
Comments addressed, please re-check. Thanks! |
|
Friendly ping on this one. Thanks! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brendandburns, dominykas The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm Thanks! |
Fixes #612