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

WIP: feat: add explanation for filters #49

Closed
wants to merge 1 commit into from

Conversation

rakshitgondwal
Copy link
Member

Closes #48

πŸ“‘ Description

Changes made-

  • Added explanation for filters under explanation.
  • Removed the index.md file under explanation.
  • Fixed one typo in reference/cli/filters.md

βœ… Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

Signed-off-by: Rakshit Gondwal <rakshitgondwal3@gmail.com>
@rakshitgondwal rakshitgondwal marked this pull request as ready for review May 27, 2023 10:55
@AnaisUrlichs
Copy link
Member

Hi there, I just took a look, I think this is a great start. At the moment, you are explaining in great detail the filter command options, what I am looking for in this section, in addition, is how it is implemented in the source code -- just as an overview where people would be able to find it. I guess we can add more detail in the future. It might be worth asking in the main K8sGPT repo or on Slack for some of the other contributors to get involved. We could merge this PR for now and create a new issue based on my feedback to address that. What do you think?

Copy link
Member

@AnaisUrlichs AnaisUrlichs left a comment

Choose a reason for hiding this comment

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

Added a comment

@rakshitgondwal
Copy link
Member Author

Hi there, I just took a look, I think this is a great start. At the moment, you are explaining in great detail the filter command options, what I am looking for in this section, in addition, is how it is implemented in the source code -- just as an overview where people would be able to find it. I guess we can add more detail in the future. It might be worth asking in the main K8sGPT repo or on Slack for some of the other contributors to get involved. We could merge this PR for now and create a new issue based on my feedback to address that. What do you think?

Oh, so are you suggesting adding information on where the filter command is implemented or a certain part is implemented in the codebase?

@AnaisUrlichs
Copy link
Member

Oh, so are you suggesting adding information on where the filter command is implemented or a certain part is implemented in the codebase?
Yes, explaining the codebase in the explanation section vs. using the CLI is in the reference section

@rakshitgondwal
Copy link
Member Author

Oh, so are you suggesting adding information on where the filter command is implemented or a certain part is implemented in the codebase?
Yes, explaining the codebase in the explanation section vs. using the CLI is in the reference section

Great idea, I can do it in this PR or we can raise another issue for it too, as you say.

@AnaisUrlichs AnaisUrlichs changed the title feat: add explanation for filters WIP: feat: add explanation for filters Jun 13, 2023
@AlexsJones
Copy link
Member

Is this ready to go @rakshitgondwal ?

@rakshitgondwal
Copy link
Member Author

Actually, me and Anais discussed adding more information about how to navigate through the codebase for filters above, but we just merged #69 in which we provide an explanation for integrations, now have we moved on from this idea as I don't really see this idea being implemented in that PR?
If not, then I can add this additional information in separate PRs for both.

@AlexsJones @AnaisUrlichs

@AlexsJones AlexsJones closed this Jan 4, 2024
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.

[Feature]: Add explanation for filters
3 participants