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 dashboard sync config #90

Closed
wants to merge 1 commit into from

Conversation

AlanGreene
Copy link
Member

@AlanGreene AlanGreene commented Apr 9, 2020

Changes

tektoncd/dashboard#1215

We've added the initial _index.md file from the Dashboard docs
to the Dashboard repo, so adding the repo to the sync config.

Waiting for tektoncd/dashboard#1268 to be merged first.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

See the contribution guide
for more details.

@tekton-robot tekton-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 9, 2020
@netlify
Copy link

netlify bot commented Apr 9, 2020

Deploy request for tekton pending review.

Review with commit 6ca4340

https://app.netlify.com/sites/tekton/deploys

@tekton-robot tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 9, 2020
@AlanGreene AlanGreene marked this pull request as ready for review April 9, 2020 17:34
@tekton-robot tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 9, 2020
@AlanGreene
Copy link
Member Author

@steveodonovan @a-roberts fyi, first step to getting more dashboard content on the website and cleaning up our docs.

@tualeron
Copy link
Contributor

/kind documentation

@tekton-robot tekton-robot added the kind/documentation Categorizes issue or PR as related to documentation. label Apr 15, 2020
@a-roberts
Copy link
Member

Looks all good here

/lgtm

(I'll be impressed if my lgtm here counts from a Prow perspective 😄 )

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 16, 2020
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.

It looks good, the only concern I have is the tag that we are pulling here.
That said we may be using master for other repos too, but that's something we need to fix!

# latest version of contents.
tags:
# The name of the tag in the GitHub repository.
- name: master
Copy link
Member

Choose a reason for hiding this comment

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

Should we pull the latest released version instead of master here?

Copy link
Member Author

Choose a reason for hiding this comment

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

We're pulling master for the other projects already so I went for consistency here. It looks like there's some rudimentary support for multiple versions baked in but it just links to the project repo rather than hosting the versioned docs on the website.

We should discuss this at the docs working group and maybe the broader group too to agree a proper strategy.

Also related: #88 and #34

Copy link
Member Author

Choose a reason for hiding this comment

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

Forgot to add this after the call...

Discussed this at the docs working group on April 22nd. With our current approach, switching the sync script to pull specific release tags instead of master means we wouldn't be able to publish updated docs between releases.

Options:

  • patch version releases for doc updates if needed
  • change sync script to be more flexible (what would the strategy be?)
  • ???

Also discussed current support for multiple versions:

  • Items under 'Previous Releases' not obviously clickable at first glance
  • clicking 'Archived Versions' brings user to the project repo's 'Tags' page, not obvious how to get to desired docs from there unless familiar with the project structure (select relevant tag, find docs folder, find correct file)

Copy link
Member

@popcor255 popcor255 May 8, 2020

Choose a reason for hiding this comment

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

@afrittoli @AlanGreene PR I think this will give us the flexibility we want.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated to pull v0.6.1.4 as the default, with master listed under 'Previous releases' (maybe we should rename that section to 'Other versions' or something?)

We've added the initial `_index.md` file from the Dashboard docs
to the Dashboard repo, so adding the repo to the sync config.
@tekton-robot tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label May 18, 2020
@AlanGreene
Copy link
Member Author

@popcor255 once we resolve #130 this PR should be good to go. I'll add some details to the issue about navigation from the 'vault' view back to the main docs experience and we can decide what we want to do to improve it before enabling multiple versions.

@popcor255
Copy link
Member

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 27, 2020
@popcor255
Copy link
Member

/assign @sbwsg @vdemeester

@tekton-robot tekton-robot assigned ghost and vdemeester May 27, 2020
@AlanGreene
Copy link
Member Author

/hold

@tekton-robot tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 27, 2020
@tekton-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sbwsg
To complete the pull request process, please assign alangreene
You can assign the PR to them by writing /assign @alangreene in a comment when ready.

The full list of commands accepted by this bot can be found 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

@vdemeester
Copy link
Member

@AlanGreene what is the status ? 📦

@AlanGreene
Copy link
Member Author

There are a number of issues with the current sync approach:

  • it's easy for an individual project to break the sync for all, this has already happened a few times where files were moved/renamed and the website sync config wasn't updated
  • docs are currently tracking master, we want to change this to track the latest releases with option to view previous releases. See Changes for Sync to Multiple Releases #130 for some related discussion
  • links to edit the docs / open issues are hard to get right with the current setup, we've fixed this before but it seems broken again. Links point to the website repo instead of the individual project repos
  • links between docs are tricky too, having them work both on the website and on GitHub is difficult due to the way links are rewritten for the website (e.g. mydoc.md becomes mydoc/ on the website, and relative links from it break)

@wlynch has a proposal in #140 to give more direct control to the individual projects that will help to solve many of these.

I'm going to close this PR and we can revisit after #140 unless there's an urgent need to surface more Dashboard docs on the website.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/documentation Categorizes issue or PR as related to documentation. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants