Skip to content
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

feat: add rootComponentVCS configuration #1350

Merged
merged 7 commits into from
Jan 10, 2025

Conversation

jeremylong
Copy link
Contributor

@jkowalleck I know you indicated you would not accept adding the configuration for rootComponentVCS. However, because auto-detect can be disabled or the VCS may not be defined in the package.json - this feature is important. This feature is essential for me as the pipeline I am using injects several configurations on the fly; I may not have control over what a developer does with the package.json - but I can ensure the pipeline correctly adds the VCS if it is not defined.

In the spirit of the open-source license, I'm providing the code changes to implement the feature. I'm hoping you will accept this PR so that I do not have to maintain an internal fork of your project.

resolves #1344

@jeremylong jeremylong requested a review from a team as a code owner January 8, 2025 14:28
HISTORY.md Outdated Show resolved Hide resolved
"comment": "as detected from PackageJson property \\"bugs.url\\""
},
{
"url": "git+https://github.com/CycloneDX/cyclonedx-webpack-plugin.git#tests/integration/feature-issue1344-no-detect",
Copy link
Member

@jkowalleck jkowalleck Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this not expected to be overridden via config, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not override based on the config. I specifically stated in the documentation that it would only be set if autodetect was disabled or a VCS was not defined in the package.json. I'm okay if we change this behavior.

@jkowalleck jkowalleck added the enhancement New feature or request label Jan 8, 2025
@jkowalleck

This comment was marked as resolved.

Copy link
Member

@jkowalleck jkowalleck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some smaller nitpicks here and there but overall,
this feature seams to be good to go.

src/plugin.ts Outdated Show resolved Hide resolved
jeremylong and others added 4 commits January 9, 2025 15:12
Co-authored-by: Jan Kowalleck <[email protected]>
Signed-off-by: Jeremy Long <[email protected]>
Signed-off-by: Jeremy Long <[email protected]>
src/plugin.ts Outdated
@@ -328,6 +336,18 @@ export class CycloneDxWebpackPlugin {
)
logger.debug('Added rootComponent BuildSystem URL:', this.rootComponentBuildSystem)
}
if (typeof this.rootComponentVCS === 'string' &&
this.rootComponentVCS.length > 0 &&
!component.externalReferences.values().some(ref => ref.type === CDX.Enums.ExternalReferenceType.VCS)) {
Copy link
Member

@jkowalleck jkowalleck Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, i proposed an invalid optimization here.
TIL Iterator.prototype.some() is not available on platforms this tool intends to support.

node 14.0.0 to node 21.0.0

(new Set([1,2,3])).values().some(i => i === 1)
Uncaught TypeError: (new Set([1,2,3])).values(...).some is not a function

added a polyfill for this purpose: 7ce6946

Signed-off-by: Jan Kowalleck <[email protected]>
@jkowalleck jkowalleck self-requested a review January 10, 2025 09:59
@jkowalleck
Copy link
Member

Thank you for donating this feature, @jeremylong

@jkowalleck jkowalleck merged commit 355b429 into CycloneDX:master Jan 10, 2025
13 checks passed
@jkowalleck
Copy link
Member

@jeremylong
Copy link
Contributor Author

@jkowalleck thank you!

@jeremylong jeremylong deleted the vcs-config branch January 10, 2025 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: populate metadata.component.externalReferences VCS and build-system from common CI environment variables
2 participants