Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
ADR 44: describing support of SDPX SBOM format in konflux #213
ADR 44: describing support of SDPX SBOM format in konflux #213
Changes from 15 commits
b370bab
43df786
f7189d5
cb666f8
1a5e34a
451622a
e3bbccf
d001dc0
9203873
221805a
682e304
4de5505
26879fc
4d053a5
f84f1cf
571c093
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you fix the formatting in this table and the tables below? It's not readable in the rendered doc https://github.com/midnightercz/konflux-ci-architecture/blob/spdx-support/ADR/0044-spdx-support.md#componentpurl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this was fixed. All tables look correct to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The loss of indentation hurts readability, especially for this one
How about changing them from Markdown tables to monospace blocks? Or just wrapping the tables in
```
so that they look the same as in the source codeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar concern to #213 (comment) - this shouldn't be the authoritative decision on how to merge packages. Not all tools should do it this way, cachi2 certainly shouldn't
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we can just re-word this to make it clear that this is a sort of default suggestion for how to do the merging if you don't have more specific requirements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, good point