operator-framework / operator-courier

Build, verify and push operators
Apache License 2.0
41 stars 53 forks source link

Only parse manifest folders with valid bundle files #122

Closed jeremy-wl closed 5 years ago

jeremy-wl commented 5 years ago

We previously require the nested manifest directories to have a SemVer name. But this blocks builds from another team that uses courier in their pipeline.

So we need to relax that requirement and parse all directories even if it does not have a SemVer name.

The requirement is removed in https://github.com/operator-framework/operator-courier/pull/116 and https://github.com/operator-framework/operator-courier/pull/118 when we verify manifest directory and run flatten command. But we need to further refactor it to only parse valid manifest folders, and ignore the rest (instead of raising errors when invalid folders are parsed). In the meantime, we also need to properly identify whether the input source directory structure is flat or nested after parsing all directories and files, and perform different actions accordingly.

APIs affected:

CLI subcommands affected

Valid Nested Layout

Valid Flat Layout

jeremy-wl commented 5 years ago

/cc @kevinrizza @MartinBasti

kevinrizza commented 5 years ago

@jeremylinlin Your PR description describes the limitations of what is happening now in terms of how you wanted the code to be refactored, but it does not actually describe what has changed from the perspective of how the API is affected. Could you please explain in your description (and commit message) which edge cases this PR is attempting to resolve, and what is expected from this?

jeremy-wl commented 5 years ago

@kevinrizza Thanks for the comment! I've updated the description and commit message.

jeremy-wl commented 5 years ago

@kevinrizza Thanks for the comments! I have addressed them and please take a look for new changes.

kevinrizza commented 5 years ago

/lgtm

jeremy-wl commented 5 years ago

Hey @MartinBasti, could you please also review it when you get a chance? Thank you!

openshift-ci-robot commented 5 years ago

New changes are detected. LGTM label has been removed.

jeremy-wl commented 5 years ago

@MartinBasti Thank you very much for the comments! I have addressed all of them and please take a look when you are free.