-
Notifications
You must be signed in to change notification settings - Fork 243
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 complete devfiles to advanced guides #6388
Add complete devfiles to advanced guides #6388
Conversation
✅ Deploy Preview for odo-docusaurus-preview ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
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.
LGTM. I agree there is a lot of repetitive data, but since the Devfiles are quite different, it might be hard to factorize, IMO. Not an expert with MDX / JSX stuff, but there might be some room for reusability. So I am not sure it would be worth spending time right now trying to optimize this.
However, this makes me think of 2 things:
- Based on a recent discussion, maybe we could use each of those Devfiles as input in E2E tests, and import them as is in these docs. This way, we would be able to test them as well. And for line highlighting in the Docs, I can imagine some generic MDX Component accepting the path to the Devfile along with line numbers as input properties, so as to keep highlighting the relevant lines.
- These Advanced Guides could be simplified further now that we support parsing multi-document YAML in a single K8s component (Add support for parsing multiple k8s definition in a single Devfile K8s component #6372)
But that's another story that can be handled in separate PRs.
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.
Thanks for linking to that other issue.
/lgtm
What type of PR is this:
/kind feature
/area documentation
What does this PR do / why we need it:
This PR adds complete Devfile to all the advanced guides.
Which issue(s) this PR fixes:
Fixes #6337
PR acceptance criteria:
Unit test
Integration test
Documentation
How to test changes / Special notes to the reviewer:
Nodejs, Go, Java, Dotnet