-
Notifications
You must be signed in to change notification settings - Fork 104
CLOUDP-341448: openapi2crd unit-test and improvements #2741
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
Conversation
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
| // |
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.
why do we remove the license?
| @@ -0,0 +1,101 @@ | |||
| package generator | |||
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.
this is missing apache licenses in all new files.
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.
follow-up: This is huge, I would advise to split into smaller PRs for next iterations.
For instance, this could have been split into a still big but single focused test coverage PR, with just the bare minimum fixes to pass the tests. And another PR for other refactors, probably smaller and simpler to follow.
|
comment: We probably want to wire a CI for this so we can see the tests passing. A single workflow such as the one for CRD2Go can do the job. We only need to run unit tests after all. |
I plan to do it in a follow-up PR. I'll create a ticket for it. |
|
Any blockers to merge this? |
Summary
Proof of Work
Checklist
Reminder (Please remove this when merging)