Festo-se / cyclonedx-editor-validator

Tool for creating, modifying and validating CycloneDX SBOMs.
https://festo-se.github.io/cyclonedx-editor-validator/
GNU General Public License v3.0
16 stars 4 forks source link

feat: Make merge <input> argument optional when `--from-folder` is given #191

Closed mmarseu closed 3 weeks ago

mmarseu commented 1 month ago

Overhauls the loading of input files in invoke_merge().

This is a breaking change for two reasons:

Tests for the new behavior will be added to #157.

github-actions[bot] commented 1 month ago

Coverage

Coverage Report •
FileStmtsMissCoverMissing
__main__.py3192093%217–218, 235, 245, 664–665, 669–674, 676, 679, 689–692, 696, 863
TOTAL16739194% 

Tests Skipped Failures Errors Time
295 2 :zzz: 0 :x: 0 :fire: 5.075s :stopwatch:
mmarseu commented 1 month ago

Should the documentation be updated? It can be kept as is, since the information is not wrong, but the case of several input files is not covered. This might be worth mentioning, since using the input function, the merging order can be influenced (input is merged in order before the folder which contents are merged alphabetically)

Valid point. The documentation should be improved.

That made me think: Is there any point to requiring an input argument when --from-folder is given? Since merge order is often important, the user has to make sure to name the files in the folder correctly anyways. Then why do we force them to single out the "primary" merge input, if it could simply be the first file in the folder in alphabetical order?

CBeck-96 commented 1 month ago

Should the documentation be updated? It can be kept as is, since the information is not wrong, but the case of several input files is not covered. This might be worth mentioning, since using the input function, the merging order can be influenced (input is merged in order before the folder which contents are merged alphabetically)

Valid point. The documentation should be improved.

That made me think: Is there any point to requiring an input argument when --from-folder is given? Since merge order is often important, the user has to make sure to name the files in the folder correctly anyways. Then why do we force them to single out the "primary" merge input, if it could simply be the first file in the folder in alphabetical order?

Good point. Now that you mention it, it would be more natural to justb (want to) merge a folders contents.

mmarseu commented 1 month ago

Should the documentation be updated? It can be kept as is, since the information is not wrong, but the case of several input files is not covered. This might be worth mentioning, since using the input function, the merging order can be influenced (input is merged in order before the folder which contents are merged alphabetically)

Valid point. The documentation should be improved. That made me think: Is there any point to requiring an input argument when --from-folder is given? Since merge order is often important, the user has to make sure to name the files in the folder correctly anyways. Then why do we force them to single out the "primary" merge input, if it could simply be the first file in the folder in alphabetical order?

Good point. Now that you mention it, it would be more natural to justb (want to) merge a folders contents.

Done. Tests will be added to #157