Skip to content

Conversation

danlishka
Copy link
Member

@danlishka danlishka commented May 30, 2023

This PR aims to add support for a new material type: JUnit XML.

We know that more types will be added in the future, so I propose to extract the upload&craft code and move it to a new reusable private function uploadAndCraft. All similar materials, such as SBOMs (SPDX, CycloneDX), Artifact, and more, may use this function instead of duplicating the code in the Craft method.

Documentation is updated in this PR chainloop-dev/docs#126

closes #120

Signed-off-by: Daniel Liszka <daniel@chainloop.dev>
@danlishka danlishka marked this pull request as draft May 30, 2023 16:40
Copy link
Member

@migmartri migmartri left a comment

Choose a reason for hiding this comment

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

Cool, good start

danlishka added 2 commits May 30, 2023 23:51
…dded tests

Signed-off-by: Daniel Liszka <daniel@chainloop.dev>
…o mod tidy

Signed-off-by: Daniel Liszka <daniel@chainloop.dev>
danlishka added 2 commits May 31, 2023 12:48
…efactoring

Signed-off-by: Daniel Liszka <daniel@chainloop.dev>
…evert the comment change

Signed-off-by: Daniel Liszka <daniel@chainloop.dev>
@danlishka danlishka marked this pull request as ready for review May 31, 2023 10:55
@danlishka danlishka requested a review from migmartri May 31, 2023 11:04
@danlishka danlishka changed the title feat: Add support fot JUnit XML material type (#120) feat: Add support for JUnit XML material type (#120) May 31, 2023
Copy link
Member

@migmartri migmartri left a comment

Choose a reason for hiding this comment

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

Looks pretty good!

The only issue I'd ask review for is the need of having a non-used context. Other than that see some optional nitpicks about readability.

Thanks!

danlishka added 3 commits May 31, 2023 15:09
…pply feedback from the PR review

Signed-off-by: Daniel Liszka <daniel@chainloop.dev>
…pply feedback from the PR review, remove unnecessary tests

Signed-off-by: Daniel Liszka <daniel@chainloop.dev>
…pply feedback from the PR review

Signed-off-by: Daniel Liszka <daniel@chainloop.dev>
Copy link
Member

@migmartri migmartri left a comment

Choose a reason for hiding this comment

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

LGTM!

…pply feedback from the PR review

Signed-off-by: Daniel Liszka <daniel@chainloop.dev>
…pply feedback from the PR review

Signed-off-by: Daniel Liszka <daniel@chainloop.dev>
Copy link
Member

@migmartri migmartri left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀 it!

@danlishka danlishka merged commit a0d3ad5 into chainloop-dev:main May 31, 2023
@danlishka danlishka deleted the junit branch May 31, 2023 17:34
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.

JUnit XML material type
2 participants