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

Improve crd-extractor script #441

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

kishorviswanathan
Copy link
Contributor

  • Fetch CRDs in parallel
  • Avoid use of associative array
  • Handle space in file path

* Fetch CRDs in parallel
* Avoid use of associative array
* Handle space in file path
@eyarz
Copy link
Member

eyarz commented Dec 23, 2024

@kishorviswanathan thank you for improving the script!
did you have a chance to run some tests with your version of the script?

@kishorviswanathan
Copy link
Contributor Author

@eyarz The networking CRDs I extracted from GKE was using the updated script. It worked fine. I don't have access to any other clusters, so I am not really sure how to test it. Do you have something in mind ?

@eyarz
Copy link
Member

eyarz commented Dec 23, 2024

@userbradley @ElanHasson can I get you help in reviewing this?
not a bash expert here and because this is a core utility I want to make sure someone will have a second look here before merging :)

@onedr0p
Copy link

onedr0p commented Dec 29, 2024

FWIW I was able to test this and runs it just fine.

@eyarz
Copy link
Member

eyarz commented Dec 29, 2024

@onedr0p thank you for the feedback!
I will wait for one more reviewer before merging it because it's a critical component in the repo

@ElanHasson
Copy link
Collaborator

Looking at this now

@ElanHasson
Copy link
Collaborator

ElanHasson commented Jan 13, 2025

@onedr0p thanks for the PR!

One of the things is see often is (example)

Maintainer: "How did you generate this PR?"
PR Creator:  [something other than the extractor]

I wonder if there is a way for the tool to "mark" the files as having been created by it.

I just made #449 to help with this

@ElanHasson ElanHasson merged commit 2c957db into datreeio:main Jan 13, 2025
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.

4 participants