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

Add missing frontmatter to some docs #3958

Merged
merged 1 commit into from
May 26, 2021

Conversation

eliasnorrby
Copy link
Contributor

@eliasnorrby eliasnorrby commented May 18, 2021

See tektoncd/website#271 for more background.

Changes

Some documentation files had no frontmatter block, making them invisible
in the sidebar navigation on the website (since they were missing the
linkTitle property).

Update weights to adjust the order of links:

  • Installation to the top
  • Deprecations at the bottom
  • Migration guides just before Deprecations

When finalized, these changes will have to be merged into the release branches of previous releases in order to keep the documentation layout consistent (again, see tektoncd/website#271).

Submitter Checklist

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

  • Docs included if any changes are user facing
  • Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including
    functionality, content, code)
  • Release notes block below has been filled in or deleted (only if no user facing changes)

Release Notes

NONE

@tekton-robot tekton-robot added the release-note-none Denotes a PR that doesnt merit a release note. label May 18, 2021
@tekton-robot tekton-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 18, 2021
@tekton-robot
Copy link
Collaborator

Hi @eliasnorrby. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@eliasnorrby
Copy link
Contributor Author

/kind documentation
/assign @pritidesai @afrittoli

@tekton-robot tekton-robot added the kind/documentation Categorizes issue or PR as related to documentation. label May 18, 2021
@afrittoli
Copy link
Member

/ok-to-test

@tekton-robot tekton-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 18, 2021
@afrittoli
Copy link
Member

/test check-pr-has-kind-label

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.

Thank you! A small update please

docs/install.md Outdated
<!--
---
linkTitle: "Installation"
weight: 1
Copy link
Member

Choose a reason for hiding this comment

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

"1" is already used by tasks.md - so maybe we can change that two 2?
There are already some details on the install process in https://tekton.dev/docs/getting-started/#installation - those are in the website repo though... so maybe we could add a link in there to this document too?

Copy link
Contributor Author

@eliasnorrby eliasnorrby May 18, 2021

Choose a reason for hiding this comment

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

I'm glad you noticed! At first, I used 1 for install.md and bumped all the other weights. But the thing is, there are multiple documents with with 2 already. Seeing this, I took the lazy approach of using 1 for both tasks.md and install.md, figuring they would be sorted alphabetically (as seems to be the case for the 2s). But I can just as well take another pass and make sure weights are unique.

Then again, it's a weight and not an order property. Both docs relating to contracts have weight: 8, which makes sense: they don't need to be in a specific order (I guess), but they should be grouped together. The problem is we're using too narrow a range, so it's hard to insert something between items.

Taking a closer look at the order as is now, it's a bit weird. I feel it would be more natural to have

  • Install
  • Task
  • TaskRun
  • Pipeline
  • PipelineRun

as the components are ordered on the index page, not

  • Install
  • Task
  • Events
  • Runs < -- experimental feature
  • TaskRuns
  • Pipeline
  • PipelineRun

as it is now.

Really going down the rabbit hole with this one, sorry. 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As to the getting-started page: sure! Something like:

For more detailed installation instructions, see Installation.

added to the bottom of the Installation section?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's a stab at a new set of weights. I tried ordering core components at the top, but I'm no Tekton expert yet, so let me know if something seems out of place.

install.md                                      weight: 1
tasks.md                                        weight: 2
taskruns.md                                     weight: 3
pipelines.md                                    weight: 4
pipelineruns.md                                 weight: 5
resources.md                                    weight: 6
workspaces.md                                   weight: 7
events.md                                       weight: 8
runs.md                                         weight: 9
auth.md                                         weight: 10
container-contract.md                           weight: 11
tekton-bundle-contracts.md                      weight: 12
logs.md                                         weight: 13
labels.md                                       weight: 14
conditions.md                                   weight: 15
podtemplates.md                                 weight: 16
enabling-ha.md                                  weight: 17
metrics.md                                      weight: 18
variables.md                                    weight: 19
tekton-controller-performance-configuration.md  weight: 20
migrating-from-knative-build.md                 weight: 40
migrating-v1alpha1-to-v1beta1.md                weight: 40
deprecations.md                                 weight: 50

I'm bumping the weight values for deprecations and migration guides (so they will stay at the bottom should any new docs be added in the future).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think about this suggestion @afrittoli? I suppose pushing an update will require new test runs, so.. should I go ahead with this?

Copy link
Member

Choose a reason for hiding this comment

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

@tektoncd/core-maintainers

Copy link

Choose a reason for hiding this comment

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

Suggest pushing resources.md down to the 15-20 range and pulling variables.md up to the 10-15 range since PipelineResources are deprecated and variables are used everywhere. WDYT @afrittoli ?

Also: it might make future updates easier if the weights are wider apart (e.g. 100, 200, ..., 1000, 1100)? We wouldn't need to re-shuffle when a new addition is made between two existing entries, it could just use the mid-weight between two existing documents (e.g. 100, 150 (new), 200).

But I don't feel strongly about either of these - lgtm otherwise, cheers!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea! Pushed a new commit applying you suggestions.

New weights
install.md                                      weight: 100
tasks.md                                        weight: 200
taskruns.md                                     weight: 300
pipelines.md                                    weight: 400
pipelineruns.md                                 weight: 500
workspaces.md                                   weight: 600
events.md                                       weight: 700
runs.md                                         weight: 800
variables.md                                    weight: 900 // higher up
auth.md                                         weight: 1000
logs.md                                         weight: 1100 // logs and metrics together
metrics.md                                      weight: 1200
labels.md                                       weight: 1300
podtemplates.md                                 weight: 1400
enabling-ha.md                                  weight: 1500
tekton-controller-performance-configuration.md  weight: 1600
container-contract.md                           weight: 1700 // contracts shifted downward
tekton-bundle-contracts.md                      weight: 1800
resources.md                                    weight: 2000 // deprecated, further down
conditions.md                                   weight: 2100 // deprecated, further down
migrating-v1alpha1-to-v1beta1.md                weight: 4000
migrating-from-knative-build.md                 weight: 4100
deprecations.md                                 weight: 5000

@pritidesai
Copy link
Member

/test pull-tekton-pipeline-alpha-integration-tests

1 similar comment
@afrittoli
Copy link
Member

/test pull-tekton-pipeline-alpha-integration-tests

@eliasnorrby eliasnorrby force-pushed the add-missing-frontmatter branch from e67d780 to 6de7d0c Compare May 22, 2021 20:28
Some documentation files had no frontmatter block, making them invisible
in the sidebar navigation on the website (since they were missing the
linkTitle property). Fixed by adding the frontmatter block.

Additionally, update weights to adjust the order of links:
- Installation at the top
- Deprecations at the bottom
- Migration guides just before Deprecations

Increase weight values to facilitate insertion of future documents.
@eliasnorrby eliasnorrby force-pushed the add-missing-frontmatter branch from 6de7d0c to 780f659 Compare May 22, 2021 20:40
@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbwsg

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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 24, 2021
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 a lot @eliasnorrby for this, it's really helpful.
The new order looks good to me. Let merge this and see how it looks on the website :)
Would you be up for backporting to the past few releases?
/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 26, 2021
@afrittoli
Copy link
Member

/retest

@tekton-robot tekton-robot merged commit 48c867b into tektoncd:main May 26, 2021
@eliasnorrby
Copy link
Contributor Author

Would you be up for backporting to the past few releases?

@afrittoli Yeah, sure! I wrote this little helper script for backporting a PRs changes to previous releases. I'll be running it against the release branches that are available and will let you know in tektoncd/website#271 if I'm missing any!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesnt merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants