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

docs: add content tabs in code examples in reference section #3005

Merged
merged 24 commits into from
Feb 20, 2024

Conversation

vishalvivekm
Copy link
Contributor

@vishalvivekm vishalvivekm commented Feb 9, 2024

Description

We've a bunch of code snippets / examples on KeptnMetricsProvider and KeptnTaskDefinition CRD reference pages that are listed vertically.

  • This PR groups related examples in tabs as instructed in the issue.
  • Creates slugified anchor tags for each tab ensuring we don't have any broken reference on the page.
slugify: !!python/object/apply:pymdownx.slugs.slugify
       kwds:
         case: lower  

Here's the Video Preview:
Screencast from 09-02-24 04:18:31 PM IST.webm

Fixes #2922

Checklist

  • My PR fulfills the Definition of Done of the corresponding issue and not more (or parts if the issue is separated
    into multiple PRs)
  • I used descriptive commit messages to help reviewers understand my thought process
  • I signed off all my commits according to the Developer Certificate of Origin (DCO)
    see Contribution Guide
  • My PR title is formatted according to the semantic PR conventions described in
    the Contribution Guide
  • My code follows the style guidelines of this project (golangci-lint passes, YAMLLint passes)
  • I have performed a self-review of my code
  • My changes result in all-green PR checks (first-time contributors need to ask a maintainer to approve their test runs)

Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
@vishalvivekm vishalvivekm requested review from a team as code owners February 9, 2024 11:12
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Feb 9, 2024
@vishalvivekm vishalvivekm changed the title add-content-tabs-in-code-examples-in-reference-section feat: add-content-tabs-in-code-examples-in-reference-section Feb 9, 2024
@vishalvivekm
Copy link
Contributor Author

vishalvivekm commented Feb 9, 2024

Hi @mowies, I've made the required changes.
The spell checker actions is failing: kwds is not a recognized word.
image

This can probably be ignored ;)

@odubajDT
Copy link
Contributor

odubajDT commented Feb 9, 2024

Hi @mowies, I've made the required changes. The spell checker actions is failing: kwds is not a recognized word. image

This can be ignored ;)

Hi @vishalvivekm thank you for contribution! :)

Please add kwds into the recognized words list. The list is present in .github/actions/spelling/expect.txt

Thank you!

Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
@github-actions github-actions bot added the ops label Feb 9, 2024
@vishalvivekm
Copy link
Contributor Author

@mowies I've done the required changes, the Spell Checker workflow is failing though.

Hi @mowies, I've made the required changes. The spell checker actions is failing: kwds is not a recognized word. image
This can be ignored ;)

Hi @vishalvivekm thank you for contribution! :)

Please add kwds into the recognized words list. The list is present in .github/actions/spelling/expect.txt

Thank you!

Thanks @odubajDT, it was helpful and all checks are passing! :)

@odubajDT
Copy link
Contributor

odubajDT commented Feb 9, 2024

@mowies I've done the required changes, the Spell Checker workflow is failing though.

Hi @mowies, I've made the required changes. The spell checker actions is failing: kwds is not a recognized word. image
This can be ignored ;)

Hi @vishalvivekm thank you for contribution! :)
Please add kwds into the recognized words list. The list is present in .github/actions/spelling/expect.txt
Thank you!

Thanks @odubajDT, it was helpful and all checks are passing! :)

No problem :) Can you please have a look also on the failing yaml and markdown lint checks? Thank you!

Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
@vishalvivekm
Copy link
Contributor Author

vishalvivekm commented Feb 9, 2024

No problem :) Can you please have a look also on the failing yaml and markdown lint checks? Thank you!

Thank you! I fixed the yaml one.

For Markdown lint checks:
There are two types of issues that the action highlightes:

    1. MD051/link-fragments Link fragments should be valid [Context: "[Example 3: functionRef for a python-runner runner](#example-3-functionref-for-a-python-runtime-runner)"]
      It's happening due to the changes like :

Before: ### Example 3: functionRef for a python-runtime runner
After: === "Example 3: functionRef for a python-runtime runner"

These aren't really an issue in my opinion as the extension:

  - pymdownx.tabbed:
      alternate_style: true
      slugify: !!python/object/apply:pymdownx.slugs.slugify
        kwds:
          case: lower

generates the corresponding anchor tags ( i.e. in this case example-3-functionref-for-a-python-runtime-runner) for each content tab on runtime and I've checked 'em all.

  • 2nd type: [Expected: fenced; Actual: indented]
    So for content tab, mkdocs necessarily requires same indentation for either plain text or code snippets in markdown.
    Now, if we fence the plain text too, we'll lose the original formatting and it will look like this:
    image

Ideally and ( currently in the build preview ) it looks like :
Screenshot from 2024-02-09 18-08-01

…pages

Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
@vishalvivekm
Copy link
Contributor Author

Given the explanation above, I think it makes sense to go ahead and disable these specific markdown lint checks in the sections of the two pages, where changes are made.

  • [MD051] link-fragments - Link fragments should be valid
  • [MD046] code-block-style - Code block style

Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
@odubajDT odubajDT changed the title feat: add-content-tabs-in-code-examples-in-reference-section docs: add content tabs in code examples in reference section Feb 11, 2024
Copy link
Contributor

@StackScribe StackScribe left a comment

Choose a reason for hiding this comment

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

Nice job on the coding! Now comes the question of how the examples should be arranged on the page. Right now, this gives us all the examples for each runner in one section, which is how the page was arranged. But there is some sentiment to group the examples by functionality with tabs for each runner.

I think we should merge this PR using the existing structure then have a separate issue that we can discuss and then implement in a new PR if we want to restructure the content.

vishalvivekm and others added 2 commits February 14, 2024 19:44
Signed-off-by: Vivek Vishal <vishalvivek488@gmail.com>
Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
@odubajDT
Copy link
Contributor

Please let's try to fix the MD linter errors. Here are the logs from the test

docs/docs/reference/crd-reference/taskdefinition.md:38:1 MD007/ul-indent Unordered list indentation [Expected: 4; Actual: 2]
docs/docs/reference/crd-reference/taskdefinition.md:47:5 MD033/no-inline-html Inline HTML [Element: a]
docs/docs/reference/crd-reference/taskdefinition.md:50:1 MD007/ul-indent Unordered list indentation [Expected: 4; Actual: 2]
docs/docs/reference/crd-reference/taskdefinition.md:54:5 MD033/no-inline-html Inline HTML [Element: a]
docs/docs/reference/crd-reference/taskdefinition.md:92:11 MD033/no-inline-html Inline HTML [Element: a]
docs/docs/reference/crd-reference/taskdefinition.md:96:11 MD033/no-inline-html Inline HTML [Element: a]
docs/docs/reference/crd-reference/taskdefinition.md:209:23 MD033/no-inline-html Inline HTML [Element: a]
docs/docs/reference/crd-reference/taskdefinition.md:212:121 MD013/line-length Line length [Expected: 120; Actual: 137]
docs/docs/reference/crd-reference/taskdefinition.md:212:23 MD033/no-inline-html Inline HTML [Element: a]
docs/docs/reference/crd-reference/taskdefinition.md:228:23 MD033/no-inline-html Inline HTML [Element: a]
docs/docs/reference/crd-reference/taskdefinition.md:230:23 MD033/no-inline-html Inline HTML [Element: a]
docs/docs/reference/crd-reference/taskdefinition.md:257:23 MD033/no-inline-html Inline HTML [Element: a]
docs/docs/reference/crd-reference/taskdefinition.md:259:121 MD013/line-length Line length [Expected: 120; Actual: 137]
docs/docs/reference/crd-reference/taskdefinition.md:259:23 MD033/no-inline-html Inline HTML [Element: a]
docs/docs/reference/crd-reference/taskdefinition.md:265:23 MD033/no-inline-html Inline HTML [Element: a]
docs/docs/reference/crd-reference/taskdefinition.md:267:121 MD013/line-length Line length [Expected: 120; Actual: 139]
docs/docs/reference/crd-reference/taskdefinition.md:267:23 MD033/no-inline-html Inline HTML [Element: a]
docs/docs/reference/crd-reference/taskdefinition.md:280:19 MD033/no-inline-html Inline HTML [Element: a]
docs/docs/reference/crd-reference/taskdefinition.md:282:121 MD013/line-length Line length [Expected: 120; Actual: 132]
docs/docs/reference/crd-reference/taskdefinition.md:282:19 MD033/no-inline-html Inline HTML [Element: a]
docs/docs/reference/crd-reference/taskdefinition.md:297:19 MD033/no-inline-html Inline HTML [Element: a]
docs/docs/reference/crd-reference/taskdefinition.md:299:121 MD013/line-length Line length [Expected: 120; Actual: 132]
docs/docs/reference/crd-reference/taskdefinition.md:299:19 MD033/no-inline-html Inline HTML [Element: a]

@vishalvivekm
Copy link
Contributor Author

MD033/no-inline-html Inline HTML [Element: a]

Hi, to avoid the MD051( why is this happening I've explained in this comment: #3005 (comment) ):

MD051/link-fragments Link fragments should be valid 
[Context: "[Example 3: functionRef for a python-runner runner](#example-3-functionref-for-a-python-runtime-runner)"]

I've used html anchor tags like this:

<a href="#example-3-functionref-for-a-python-runtime-runner">Example 3: functionRef for a python-runtime runner</a>

which is now causing MD033/no-inline-html Inline HTML [Element: a] and at some places: MD013/line-length Line length as the elements have length >= 120 .
We'll have to allow <a> in the markdown, can I disable these linters for the anchor tags in the page using <!-- markdownlint-disable --> syntax, @odubajDT ?

Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
@odubajDT
Copy link
Contributor

MD033/no-inline-html Inline HTML [Element: a]

Hi, to avoid the MD051( why is this happening I've explained in this comment: #3005 (comment) ):

MD051/link-fragments Link fragments should be valid 
[Context: "[Example 3: functionRef for a python-runner runner](#example-3-functionref-for-a-python-runtime-runner)"]

I've used html anchor tags like this:

<a href="#example-3-functionref-for-a-python-runtime-runner">Example 3: functionRef for a python-runtime runner</a>

which is now causing MD033/no-inline-html Inline HTML [Element: a] and at some places: MD013/line-length Line length as the elements have length >= 120 . We'll have to allow <a> in the markdown, can I disable these linters for the anchor tags in the page using <!-- markdownlint-disable --> syntax, @odubajDT ?

@vishalvivekm let's try to add the element to the list of allowed elements here and see if it works for our use-case.

Regarding the line length, it should be ok to disable the line-length markdownlint check but only for that particular lines. Let's please not do that for bigger blocks.

…sing include

Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
Copy link

codecov bot commented Feb 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ce57993) 85.75% compared to head (c5006e6) 86.77%.
Report is 47 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3005      +/-   ##
==========================================
+ Coverage   85.75%   86.77%   +1.02%     
==========================================
  Files         162      162              
  Lines       10351     8554    -1797     
==========================================
- Hits         8876     7423    -1453     
+ Misses       1186      841     -345     
- Partials      289      290       +1     

see 141 files with indirect coverage changes

Flag Coverage Δ
certificate-operator 69.23% <ø> (+1.80%) ⬆️
component-tests 56.64% <ø> (+0.64%) ⬆️
keptn-lifecycle-operator ?
lifecycle-operator 86.40% <ø> (+1.11%) ⬆️
metrics-operator 88.32% <ø> (+0.68%) ⬆️
scheduler 34.74% <ø> (-1.79%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@vishalvivekm
Copy link
Contributor Author

All done @odubajDT

odubajDT
odubajDT previously approved these changes Feb 19, 2024
Copy link
Contributor

@odubajDT odubajDT left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!

@vishalvivekm
Copy link
Contributor Author

Thank you for your contribution!

😄
Thanks to you too, for offering your help and additional resources 😎

Copy link
Contributor

@RealAnna RealAnna left a comment

Choose a reason for hiding this comment

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

@vishalvivekm this looks already so much better 🚀 ! I was wondering if it makes sense to tab by example instead of by runtime. Most of the example use cases are present for both runtimes, and I think that would be an easier way to see each possible configuration...

eg.

Inline ex (both python and deno)
Httpref ex (both)

and so on...

It is a bit difficult to search the page for specific examples right now

@vishalvivekm
Copy link
Contributor Author

@vishalvivekm this looks already so much better 🚀 ! I was wondering if it makes sense to tab by example instead of by runtime. Most of the example use cases are present for both runtimes, and I think that would be an easier way to see each possible configuration...

eg.

Inline ex (both python and deno) Httpref ex (both)

and so on...

It is a bit difficult to search the page for specific examples right now

Makes sense @RealAnna , thank you for the review.
I'll go ahead and make changes.

Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
@vishalvivekm
Copy link
Contributor Author

@vishalvivekm this looks already so much better 🚀 ! I was wondering if it makes sense to tab by example instead of by runtime. Most of the example use cases are present for both runtimes, and I think that would be an easier way to see each possible configuration...
eg.
Inline ex (both python and deno) Httpref ex (both)
and so on...
It is a bit difficult to search the page for specific examples right now

Makes sense @RealAnna , thank you for the review. I'll go ahead and make changes.

done ✔️

Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
Copy link

sonarcloud bot commented Feb 19, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link
Contributor

@RealAnna RealAnna left a comment

Choose a reason for hiding this comment

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

very nice 🚀 💪 🎊

@odubajDT odubajDT merged commit cf0c170 into keptn:main Feb 20, 2024
34 checks passed
Bharadwajshivam28 pushed a commit to Bharadwajshivam28/lifecycle-toolkit that referenced this pull request Feb 20, 2024
)

Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
Signed-off-by: Vivek Vishal <vishalvivek488@gmail.com>
Signed-off-by: shivam <shivambharadwaj822@gmail.com>
Vickysomtee pushed a commit to Vickysomtee/keptn-lifecycle-toolkit that referenced this pull request Apr 23, 2024
)

Signed-off-by: vishalvivekm <110674407+vishalvivekm@users.noreply.github.com>
Signed-off-by: Vivek Vishal <vishalvivek488@gmail.com>
Signed-off-by: vickysomtee <vickysomtee@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation ops
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use content tabs in CRD reference code examples
4 participants