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

fix file extension for cyclonedx #974

Merged
merged 1 commit into from
Mar 9, 2023

Conversation

developer-guy
Copy link
Collaborator

PTAL @imjasonh

Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com>
@codecov-commenter
Copy link

Codecov Report

Merging #974 (15c5757) into main (7ce9478) will not change coverage.
The diff coverage is 0.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main     #974   +/-   ##
=======================================
  Coverage   52.42%   52.42%           
=======================================
  Files          43       43           
  Lines        3325     3325           
=======================================
  Hits         1743     1743           
  Misses       1353     1353           
  Partials      229      229           
Impacted Files Coverage Δ
pkg/build/gobuild.go 57.07% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@imjasonh
Copy link
Member

imjasonh commented Mar 8, 2023

Is this incorrect as specified today? Is there some code that expects it to be named cyclonedx.json specifically?

@developer-guy
Copy link
Collaborator Author

developer-guy commented Mar 8, 2023

Is this incorrect as specified today?

yes, this is a typo.

Is there some code that expects it to be named cyclonedx.json specifically?

nope, this is just a convention we define to specify the SBOM type within the file

@imjasonh
Copy link
Member

imjasonh commented Mar 8, 2023

I guess the problem is, now, someone somewhere might be depending on the cyclone.json that ko produces, and changing it, even if it's to match the convention, might break them. There's no way to know.

I'm fine making the change, to be clear. ko is pre-1.0, and makes no stability guarantees. Probably nobody uses cyclonedx SBOMs written to disk anyway, since that's not the default behavior. But we should take care not to risk breaking folks unless there's a clear benefit. Maybe matching the convention is enough benefit?

@developer-guy
Copy link
Collaborator Author

Probably nobody uses cyclonedx SBOMs written to disk anyway,

I agree because we didn't release that support yet either, this is why I'm not concerned about breaking anything unless someone uses ko from its main.

Maybe matching the convention is enough benefit?

yes because this is not the correct naming for the file, we made a typo and we should fix this.

Btw, thanks for bringing these concerns to my attention!

@imjasonh imjasonh merged commit 8e075ae into ko-build:main Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants