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: extract code samples to shortcodes #2633

Closed
wants to merge 1 commit into from

Conversation

ahmedavid
Copy link
Contributor

@ahmedavid ahmedavid commented Dec 5, 2023

Description

Extract code samples to shortcodes

Fixes #2587

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Dec 5, 2023
Copy link

netlify bot commented Dec 5, 2023

Deploy Preview for keptn-lifecycle-toolkit ready!

Name Link
🔨 Latest commit eb985a7
🔍 Latest deploy log https://app.netlify.com/sites/keptn-lifecycle-toolkit/deploys/657013b645af4700084f221e
😎 Deploy Preview https://deploy-preview-2633--keptn-lifecycle-toolkit.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@mowies mowies changed the title Extract code samples to shortcodes docs: extract code samples to shortcodes Dec 5, 2023
@mowies
Copy link
Member

mowies commented Dec 5, 2023

First of all, you're not up to date with the latest changes on main branch ;)
Also, I think it would make the most sense to only extract the code from yaml code blocks. The rest (mostly shell), just doesn't really make sense in external files.

@ahmedavid
Copy link
Contributor Author

@mowies , how about now?

Comment on lines 14 to 21
'-X',
'POST',
'http://webhook.webhook.svc.cluster.local:8084/YOUR-UUID-HERE',
'-H',
'Content-Type: application/json',
'-d',
'{ "from": "keptn send-event" }'
]
Copy link
Contributor

Choose a reason for hiding this comment

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

If possible, please increase the indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @odubajDT , what indentation is preferred 2 or 4 spaces?

Copy link
Contributor

@odubajDT odubajDT Dec 6, 2023

Choose a reason for hiding this comment

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

in this particular case 2 spaces more, you can find all the details here https://github.com/keptn/lifecycle-toolkit/actions/runs/7111043259/job/19360404602?pr=2633

@odubajDT
Copy link
Contributor

odubajDT commented Dec 5, 2023

Thank you for your contribution, generally everything looks fine. Can you please adapt the yaml files to make the yamllint check pass? Generally it's mostly newlines or wrong indentation. You can find the results here https://github.com/keptn/lifecycle-toolkit/actions/runs/7097569330/job/19331781969?pr=2633

Thanks!

@odubajDT
Copy link
Contributor

odubajDT commented Dec 6, 2023

Hi @ahmedavid can you please rename the newly created yaml files so that the names will reflect the pages that they are used in and also possibly a use case they are showing? Naming the files with numbers will make it hard in the future to maintain and create connections between docs and resources referenced in the specific parts. Thanks!

namespace: simplenode-dev
evaluationTarget: ">4"
```
{% include "usecase-orchestrate.md_1.yaml" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

does the include actually work? The resource is located in assets folder, so I guess the path should be assets/usecase-orchestrate.md_1.yaml. You can try it out locally by executing make docs-build && make docs-serve

@odubajDT
Copy link
Contributor

odubajDT commented Dec 6, 2023

Also I apology for the misunderstanding, but this change should be made in the docs-new folder and not the docs/ folder. Can you please adapt it as well? Thank you!

@ahmedavid
Copy link
Contributor Author

Hi, how can I run yaml linter locally?

@odubajDT
Copy link
Contributor

Hi, how can I run yaml linter locally?

Hi make yamllint should do the trick

@ahmedavid
Copy link
Contributor Author

Hi, how can I run yaml linter locally?

Hi make yamllint should do the trick

Hello, formatting issues stem from original files. I can't fix them manually. Is it possible to use some kind of auto formatter? Please recommend one.

@odubajDT
Copy link
Contributor

Hello @ahmedavid any updates on this please ?

@ahmedavid
Copy link
Contributor Author

Hello @ahmedavid any updates on this please ?

Hello, yes please refer to previous message

@odubajDT
Copy link
Contributor

We would recommend this tool https://github.com/cytopia/docker-yamllint

What problems do you face exactly ?

@RealAnna
Copy link
Contributor

RealAnna commented Dec 21, 2023

Hi @ahmedavid please drop any changes under docs/content/en/docs these folders will be deleted soon and do not require changes, only the docs-new path does. Also make sure to rebase the changes from main to resolve current conflicts.

@ahmedavid ahmedavid closed this Dec 22, 2023
@ahmedavid ahmedavid force-pushed the feat/extract-shortcodes branch from 68942a7 to 9ee4583 Compare December 22, 2023 09:25
Signed-off-by: David Ahmadov <[email protected]>
@ahmedavid ahmedavid reopened this Dec 22, 2023
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

Please retry analysis of this Pull-Request directly on SonarCloud

'{ "from": "keptn send-event" }'
]
```
{% include "./assets/lifecycle-management.md_1.yaml" %}
Copy link
Contributor

@odubajDT odubajDT Jan 5, 2024

Choose a reason for hiding this comment

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

@odubajDT
Copy link
Contributor

odubajDT commented Jan 5, 2024

Hi @ahmedavid can you please resolve the conflicts and adapt the docs according to this comment?

Thank you!

@odubajDT
Copy link
Contributor

This PR is opened for 3 weeks without any activity, closing it and unassigning the author from the ticket. If you would like to continue, please feel free to open a new PR!

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Substitute code fragments with include shortcodes
4 participants