feat: decouple versions from attributions file #5
Conversation
| * | ||
| * @default true | ||
| */ | ||
| readonly sourcemap?: boolean; |
There was a problem hiding this comment.
Added this option to be able to write deterministic snapshot tests.
| .option('test', { type: 'string', desc: 'Validation command to sanity test the bundle after its created' }) | ||
| .command('validate', 'Validate the package is ready for bundling', args => args | ||
| .option('fix', { type: 'boolean', default: false, alias: 'f', desc: 'Fix any fixable violations' }), | ||
| ) | ||
| .command('write', 'Write the bundled version of the project to a temp directory') | ||
| .command('pack', 'Write the bundle and create the tarball') | ||
| .command('pack', 'Write the bundle and create the tarball', args => args | ||
| .option('target', { type: 'string', desc: 'Target directory of the tarball' }), |
There was a problem hiding this comment.
I noticed this option was missing from the CLI, so decided to add it here since it going to be needed fairly soon.
| }); | ||
|
|
||
| const bundleDir = bundle.write(); | ||
|
|
||
| expect(fs.existsSync(path.join(bundleDir, pkg.entrypoint))).toBeTruthy(); | ||
| expect(fs.existsSync(path.join(bundleDir, 'package.json'))).toBeTruthy(); | ||
| expect(fs.existsSync(path.join(bundleDir, 'THIRD_PARTY_LICENSES'))).toBeTruthy(); |
There was a problem hiding this comment.
This should never have been here, the tests used to mistakenly create empty attribution files, and this test specifically doesn't do validation.
|
|
||
| expect(entrypoint).toMatchSnapshot(); |
There was a problem hiding this comment.
This asserts that the entry-point is actually a result of bundling
|
|
||
| expect(entrypoint).toMatchSnapshot(); | ||
| expect(Object.keys(manifest.devDependencies)).toEqual(['dep1', 'dep2']); |
There was a problem hiding this comment.
Just a missing assertion
| const attributions = [ | ||
| 'The consumer package includes the following third-party software/licensing:', | ||
| '', | ||
| `** ${dep1.name}@${dep1.version} - https://www.npmjs.com/package/${dep1.name}/v/${dep1.version} | MIT`, | ||
| '', | ||
| '----------------', | ||
| '', | ||
| `** ${dep2.name}@${dep2.version} - https://www.npmjs.com/package/${dep2.name}/v/${dep2.version} | Apache-2.0`, | ||
| '', | ||
| '----------------', | ||
| '', | ||
| ]; |
There was a problem hiding this comment.
All of this is simply not needed, I was trying to manually create the expected attribution file so the test passes validation, instead of simply generating it as part of the test.
|
|
||
| test('validate and fix', () => { |
There was a problem hiding this comment.
Removed since validation with fix is now called (and asserted on) from the packing test
Currently, the attributions document we generate contains version information of the dependency. For example:
** @tootallnate/[email protected] - https://www.npmjs.com/package/@tootallnate/once/v/1.1.2 | MITThis creates some rough edges around automatic dependency upgrades. When dependencies are upgraded, the versions naturally change, this means that the attributions need to be regenerated (otherwise validation will fail). Since dependency upgrades are usually automatically approved, any changes to the attribution document are merged without human review, which goes against our best practice recommendation.
A solution could be to disable auto approval of dependency upgrades, but this creates a significant maintenance burden. In practice, we are only trying to avoid automatically merging substantial changes to the attribution document, where substantial means any change that isn't simply a version bump. (e.g license has changed, new transitive dependency added...)
The solution proposed in this PR is to split the attributions in two:
THIRD_PARTY_LICENSESfile to hold license information without versions.THIRD_PARTY_LICENSES.versions.jsonfile that contains which versions are used for each dependency.The
THIRD_PARTY_LICENSESis checked into source control and validated as is now, but theTHIRD_PARTY_LICENSES.versions.jsonis generated on-demand at packaging time. This way, consumers are able to distinguish between PRs that actually change license information, and ones that simply bump version numbers. They can then comprise a workflow for appropriately approving / reviewing each.