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 Hands-on tutorial #18583

Merged
merged 1 commit into from
Jan 27, 2022
Merged

Conversation

vannyle
Copy link
Contributor

@vannyle vannyle commented Jan 21, 2022

This pull request adds the following text to documentation after the line Getting Started Guides:

"Hands-on tutorial in a live environment to quickly get started with Cilium."

image

Signed-off-by: Van Le vannnyle@gmail.com

@vannyle vannyle requested a review from a team as a code owner January 21, 2022 16:49
@vannyle vannyle requested a review from qmonnet January 21, 2022 16:49
@maintainer-s-little-helper
Copy link

Commit 4ca1bddcf1879a1983f4d6d4585fe52e31eef9c2 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 21, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 21, 2022
@maintainer-s-little-helper
Copy link

Commit 4ca1bddcf1879a1983f4d6d4585fe52e31eef9c2 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

1 similar comment
@maintainer-s-little-helper
Copy link

Commit 4ca1bddcf1879a1983f4d6d4585fe52e31eef9c2 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@qmonnet qmonnet added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact. labels Jan 24, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jan 24, 2022
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Hi, and thanks a lot for the PR!

I think it is a good idea to have this link on this page, because it's definitely an interesting resource to dive deeper into Cilium. However, this itemised list is specifically about the different sections of the documentation, and the Instruqt classes are not exactly part of it.

My opinion would be to move this right below the list (just above the “Getting Started“ title), as a non-indented sentence. What do you think? @joestringer (as a member of cilium/docs-structure), what would you say, I think it's OK to have this link on the front page?

On another note, can you please squash your commits into a single one, and add a Sign-off-by: tag to your commit description as explained in the automatic messages above?

@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 26, 2022
@vannyle
Copy link
Contributor Author

vannyle commented Jan 26, 2022

On another note, can you please squash your commits into a single one, and add a Sign-off-by: tag to your commit description as explained in the automatic messages above?

Hello @qmonnet, thanks a lot for the comment!
I found out about Sign-off-by: after the first commit and couldn't figure out how to fix it. Squashing commits to the single one worked.
Also, it would be great if you can help with where this link should be placed.

@qmonnet
Copy link
Member

qmonnet commented Jan 26, 2022

Thanks for editing the commit!

Also, it would be great if you can help with where this link should be placed.

I was suggesting at the end of the same section, but below the list:

doc

@maintainer-s-little-helper
Copy link

Commit 0eb2121bd3b219c7788a4cd41815be52dcb1d6fc does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 26, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 26, 2022
@vannyle vannyle requested a review from qmonnet January 26, 2022 18:56
Documentation/index.rst Outdated Show resolved Hide resolved
Documentation/index.rst Outdated Show resolved Hide resolved
@vannyle vannyle requested a review from qmonnet January 26, 2022 20:36
@qmonnet
Copy link
Member

qmonnet commented Jan 27, 2022

The changes look good to me, thank you.

Would you mind squashing your commits once more, please? Please also mind that the Signed-off-by tag is not correctly formatted: your email address should be enclosed between angle brackets, i.e. instead of this:

Signed-off-by: Van Le vannnyle@gmail.com

it should be:

Signed-off-by: Van Le <vannnyle@gmail.com>

This is what causes the Checkpatch action to fail with:

Error: ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Van Le vannnyle@gmail.com'

Add the following text to documentation:
Hands-on tutorial in a live environment to quickly get started with Cilium.

Signed-off-by: Van Le <vannnyle@gmail.com>
@vannyle
Copy link
Contributor Author

vannyle commented Jan 27, 2022

The changes look good to me, thank you.

Would you mind squashing your commits once more, please? Please also mind that the Signed-off-by tag is not correctly formatted: your email address should be enclosed between angle brackets, i.e. instead of this:

Signed-off-by: Van Le vannnyle@gmail.com

it should be:

Signed-off-by: Van Le <vannnyle@gmail.com>

This is what causes the Checkpatch action to fail with:

Error: ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Van Le vannnyle@gmail.com'

@qmonnet Thank you for the detailed comment on how to add Signed-off-by correctly!
The commits have been squashed with the correct commit message.

Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Looks all good, thank you!

@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.11.2 Jan 31, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.8 Jan 31, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.13 Jan 31, 2022
@vannyle
Copy link
Contributor Author

vannyle commented Jan 31, 2022

This means backporting the change to the latest stable branch. I have no objection with that, I'll add the relevant labels so we pick it up for the next round of backports.

Thanks for the help!

@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.10 in 1.10.8 Feb 7, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.10 in 1.11.2 Feb 7, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.8 Feb 8, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.9 in 1.9.13 Feb 8, 2022
@joamaki joamaki added backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. and removed backport-pending/1.11 labels Feb 8, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.11 in 1.11.2 Feb 8, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.11 in 1.11.2 Feb 8, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.13 Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.10.8
Backport done to v1.10
1.11.2
Backport done to v1.11
1.9.13
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

None yet

4 participants