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.

Sign up for GitHub

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

Jump to bottom

Refactor installation documentation #5806

Merged
merged 1 commit into from
Feb 7, 2023
Merged

Refactor installation documentation #5806

merged 1 commit into from
Feb 7, 2023

Conversation

geriom
Copy link
Contributor

@geriom geriom commented Nov 28, 2022

Changes

  • Simplify installation instructions
  • Move configuration docs to a separate file.
  • Include vendor specific docs for OpenShift and Google Cloud.

As discussed in the proposal for this change during the community meeting, the documentation is only rendered correctly on the website. This and the corresponding PR on the website repo must be merged in short succession.

Previews

/kind documentation

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Has Docs included if any changes are user facing
  • Has Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including
    functionality, content, code)
  • Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
  • Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
  • Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

@tekton-robot tekton-robot added kind/documentation Categorizes issue or PR as related to documentation. release-note-none Denotes a PR that doesnt merit a release note. labels Nov 28, 2022
@tekton-robot tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 28, 2022
@geriom geriom mentioned this pull request Nov 28, 2022
Merged
2 tasks
@bendory
Copy link
Contributor

bendory commented Nov 30, 2022

Forgive my ignorance if this is a newbie question... is there a staging area where I can see the rendered website as it will appear after the merge?

lbernick
lbernick approved these changes Nov 30, 2022
View reviewed changes
Copy link
Member

@lbernick lbernick left a comment

Choose a reason for hiding this comment

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

Thanks Geri! I really like the tabs for installing on different providers/with different registries!

docs/install.md Outdated Show resolved Hide resolved
docs/install.md Show resolved Hide resolved
@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 30, 2022
@geriom
Copy link
Contributor Author

geriom commented Dec 1, 2022 •
edited
Loading

Forgive my ignorance if this is a newbie question... is there a staging area where I can see the rendered website as it will appear after the merge?

@bendory The links in the "Preview" section take you there. For changes outside the website repo the preview is not autogenerated, I created a separate PR just to generate the preview.

@bendory
Copy link
Contributor

bendory commented Dec 1, 2022

Forgive my ignorance if this is a newbie question... is there a staging area where I can see the rendered website as it will appear after the merge?

@bendory The links in the "Preview" section take you there. For changes outside the website repo the preview is not autogenerated, I created a separate PR just to generate the preview.

OIC, thank you -- these look great, I love the way you can now "choose your own adventure!"

/LGTM

@tekton-robot
Copy link
Collaborator

@bendory: changing LGTM is restricted to collaborators

In response to this:

Forgive my ignorance if this is a newbie question... is there a staging area where I can see the rendered website as it will appear after the merge?

@bendory The links in the "Preview" section take you there. For changes outside the website repo the preview is not autogenerated, I created a separate PR just to generate the preview.

OIC, thank you -- these look great, I love the way you can now "choose your own adventure!"

/LGTM

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@geriom geriom force-pushed the refactor-installation branch 2 times, most recently from b76e9d1 to c2b373e Compare December 6, 2022 15:30
@tekton-robot tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 6, 2022
@tekton-robot tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 16, 2022
@tekton-robot tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 19, 2022
@tekton-robot tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 17, 2023
@geriom
Copy link
Contributor Author

geriom commented Jan 17, 2023

/retest

jerop
jerop approved these changes Jan 17, 2023
View reviewed changes
Copy link
Member

@jerop jerop left a comment

Choose a reason for hiding this comment

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

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 17, 2023
@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerop, lbernick

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jerop
Copy link
Member

jerop commented Jan 17, 2023

/retest

@tekton-robot tekton-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm Indicates that a PR is ready to be merged. labels Jan 18, 2023
afrittoli
afrittoli reviewed Jan 20, 2023
View reviewed changes
docs/install.md Outdated Show resolved Hide resolved
afrittoli
afrittoli reviewed Jan 20, 2023
View reviewed changes
Copy link
Member

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

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

Thanks for this @geriom !
A couple of questions:

  • we have no way to see here the final rendering result of this PR. Would it make sense to have a PR against the website repo that pulls in this PR and shows the final result?
  • I like that we have tabs with different installation options. Should we have one about using the operator that points to the operator docs as well?
  • NIT: I'm not entirely sure about the "post-install" naming. The configuration is not exclusively done after the installation, the installation files can also be customised to install directly with the required configuration

docs/install.md Outdated Show resolved Hide resolved
@geriom geriom mentioned this pull request Jan 23, 2023
Open
@geriom
Copy link
Contributor Author

geriom commented Jan 23, 2023 •
edited
Loading

@afrittoli

  • we have no way to see here the final rendering result of this PR. Would it make sense to have a PR against the website repo that pulls in this PR and shows the final result?

I was thinking the same thing. For this PR I rendered it locally see preview here, but that's not a feasible solution long term. Not sure how much engineering that would require, but it's probably worth the effort. I opened an issue on the website repo to track this

  • I like that we have tabs with different installation options. Should we have one about using the operator that points to the operator docs as well?

Yes, I think that's a good idea. We can make that a separate issue and iterate this doc? since this PR is been open for two moths now.

  • NIT: I'm not entirely sure about the "post-install" naming. The configuration is not exclusively done after the installation, the installation files can also be customized to install directly with the required configuration.

Renamed the file to additional-configs.md and changed the title to "Additional configuration options", what do you think?

@tekton-robot tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 23, 2023
@tekton-robot tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 27, 2023
@geriom
Refactor installation documentation
6cf5f9c 
- Simplify installation instructions
- Move configuration docs to a separate file.
- Include vendor specific docs for OpenShift and Google Cloud.

The documentation is only rendered correctly on the website.
@tekton-robot tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 7, 2023
@afrittoli
Copy link
Member

Thanks for the updates!
/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 7, 2023
@tekton-robot tekton-robot merged commit 924ef91 into tektoncd:main Feb 7, 2023
@AlanGreene AlanGreene mentioned this pull request Feb 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afrittoli afrittoli afrittoli left review comments

@lbernick lbernick lbernick approved these changes

@jerop jerop jerop approved these changes

@imjasonh imjasonh Awaiting requested review from imjasonh

@vdemeester vdemeester Awaiting requested review from vdemeester

Assignees

@afrittoli afrittoli

@jerop jerop

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/documentation Categorizes issue or PR as related to documentation. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesnt merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@geriom @bendory @tekton-robot @jerop @afrittoli @lbernick