Skip to content
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

add support for standard Condition APIs #203

Merged
merged 2 commits into from
Mar 9, 2024

Conversation

aryan9600
Copy link
Contributor

@aryan9600 aryan9600 commented Mar 7, 2024

Detect the presence of a Conditions object in a schema and use k8s_openapi::apimachinery::pkg::apis::meta::v1::Condition if present. Introduce a new flag --no-condition which allows revering back to the old behavior of generating a custom Condition API for each instance.

Furthermore, introduce analyzer::Config which allows for customizing the behavior of the analyze function.

Fixes #199

@clux
Copy link
Member

clux commented Mar 7, 2024

wow, this is a huge change. it passes tests which is a great sign! thanks a lot.

going to try to cross-reference the code with main manually for a little while (because diff is not great with indent and code change at the same time). i'll put some comments on as i go along. if there are any particular parts of code that you would like to call attention to feel free to put any annotations/comments on the PR.

Copy link
Member

@clux clux left a comment

Choose a reason for hiding this comment

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

minor comments, and a suggestion that might avoid the indent

src/analyzer.rs Outdated Show resolved Hide resolved
src/analyzer.rs Outdated Show resolved Hide resolved
src/analyzer.rs Outdated Show resolved Hide resolved
src/analyzer.rs Show resolved Hide resolved
src/analyzer.rs Outdated Show resolved Hide resolved
@clux
Copy link
Member

clux commented Mar 7, 2024

if you don't mind i'll push a commit on top of this

@clux
Copy link
Member

clux commented Mar 7, 2024

added 3 4 commits to #204 - if you are happy with these feel free to cherry-pick them in, and i'm happy to merge :-)

@clux
Copy link
Member

clux commented Mar 8, 2024

needs a just fmt + plus cherry-pick f8286ca for integration tests

aryan9600 and others added 2 commits March 9, 2024 10:54
Detect the presence of a Conditions object in a schema and use
`k8s_openapi::apimachinery::pkg::apis::meta::v1::Condition` if present.
Introduce a new flag `--no-condition` which allows revering back to the
old behavior of generating a custom Condition API for each instance.

Furthermore, introduce `analyzer::Config` which allows for customizing
the behavior of the `analyze` function.

Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
Co-authored-by: clux <sszynrae@gmail.com>
Signed-off-by: clux <sszynrae@gmail.com>
Copy link
Member

@clux clux left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

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.

Use k8s_openapi::apimachinery::pkg::apis::meta::v1::Condition instead of generating a new API
2 participants