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

feat: extract /plugins #11464

Merged
merged 12 commits into from
Jan 11, 2022
Merged

feat: extract /plugins #11464

merged 12 commits into from
Jan 11, 2022

Conversation

thiskevinwang
Copy link
Contributor

@thiskevinwang thiskevinwang commented Dec 20, 2021

Preview

Description

This moves external plugins to a new /plugins route.

This is in support of unblocking versioned-docs (core docs).


@thiskevinwang thiskevinwang requested a review from a team as a code owner December 20, 2021 18:04
Copy link
Contributor

@zchsh zchsh left a comment

Choose a reason for hiding this comment

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

💖 This is awesome! resolve-nav-data looks way cleaner and simpler, and I'm really liking the new organization on the /plugins route.

Reviewed the diff and also pulled the branch down locally and checked things out, and I think you're good to go ahead and delete all the crufty now-commented-out code in resolve-nav-data 💯 Apart from that, and the index.mdx page (which we likely need Packer team help filling it), this feels nearly ready to ship, IMO!

website/data/docs-nav-data.json Outdated Show resolved Hide resolved
website/data/plugins-nav-data.json Outdated Show resolved Hide resolved
page_title: Plugins FIXME
---

# Plugins FIX ME
Copy link
Contributor

Choose a reason for hiding this comment

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

💖 Love that this is stubbed out

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@azr would you be able to add some text here? Or tag someone else? 🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes !

Co-authored-by: Zachary Shilton <4624598+zchsh@users.noreply.github.com>
Co-authored-by: Zachary Shilton <4624598+zchsh@users.noreply.github.com>
Copy link
Contributor

@azr azr left a comment

Choose a reason for hiding this comment

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

This is super dope, I added content to the new plugin page, from an already existing plugin page. I think it makes sense there, but these pages sound a bit duplicate now.

Maybe that section:
Screenshot 2021-12-23 at 14 24 08

Should just be about extending packer ? Or not even be here anymore 🤔

BTW, I found myself wanting to click on the version of a plugin, to sort's of go on the release page. If that's easy, that'd be nice :D

Is there a documentation somewhere on how to test documentation rendering ?

Super nice work !!!!

@azr
Copy link
Contributor

azr commented Dec 23, 2021

Oh, forgot to add:
Screenshot 2021-12-23 at 14 25 42

I think these overviews should be at the top, and not tested. They kind of have the same content.
Edit: but it sounds like this is remote-docs side and not this side, so never mind !

@thiskevinwang
Copy link
Contributor Author

@azr

BTW, I found myself wanting to click on the version of a plugin, to sort's of go on the release page. If that's easy, that'd be nice :D

Versioning for plugins is on our road map! We're aiming to handle that shortly after the core versioned-docs rollout.

I think these overviews should be at the top, and not tested. They kind of have the same content.
Edit: but it sounds like this is remote-docs side and not this side, so never mind !

Yea. These nested "Overview" pages are generated from nested index.mdx files in the plugin repos

Ex.

UI Repo
image image

I do agree though that the "Overview"s should be merged into a top-level one. We'll likely need a more solidified pattern to handle this.

Is there a documentation somewhere on how to test documentation rendering ?

@azr are you referring to manual testing? Or unit/integration testing? Or something else?
@zchsh any context from your end here?

@zchsh
Copy link
Contributor

zchsh commented Jan 3, 2022

@azr Unfortunately we don't have a super smooth workflow at present for testing rendering documentation from work-in-progress in remote plugin repos. I think @sylviamoss set up a workflow involving local .zip files so will have more insight here 👍 (From what I can tell we've retained the local .zip option with this refactor, cc @thiskevinwang in case you can confirm)

@zchsh zchsh self-requested a review January 3, 2022 18:36
Copy link
Contributor

@zchsh zchsh left a comment

Choose a reason for hiding this comment

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

@thiskevinwang Realized one missing piece here might be redirects - I think we'll want to generate redirects for any external plugins. For example, I think we want to redirect from /docs/datasources/hashicups/coffees to /plugins/datasources/hashicups/coffees.

Seems like a high-level docs/datasource/* >> plugins/datasources/* redirect would conflict with any built-in plugins... so my hot take is I think we'll need to go a level down and do eg /docs/datasources/hashicups/* >> /plugins/datasources/hashicups/* redirects. But very open to other approaches 🙇‍♂️

@thiskevinwang
Copy link
Contributor Author

@thiskevinwang Realized one missing piece here might be redirects - I think we'll want to generate redirects for any external plugins. For example, I think we want to redirect from /docs/datasources/hashicups/coffees to /plugins/datasources/hashicups/coffees.

Seems like a high-level docs/datasource/* >> plugins/datasources/* redirect would conflict with any built-in plugins... so my hot take is I think we'll need to go a level down and do eg /docs/datasources/hashicups/* >> /plugins/datasources/hashicups/* redirects. But very open to other approaches 🙇‍♂️

Ah yea, thanks for catching this @zchsh. I'll get those sorted out.

@thiskevinwang
Copy link
Contributor Author

thiskevinwang commented Jan 4, 2022

@zchsh Added redirects in efa2d0e

... I think we want to redirect from /docs/datasources/hashicups/coffees to /plugins/datasources/hashicups/coffees.

Quick smoke test: this works now

Copy link
Contributor

@azr azr left a comment

Choose a reason for hiding this comment

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

LGTM !!

Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

🙌🏽 🎉 🚀

@thiskevinwang thiskevinwang added backport/website Backport PR changes to `stable-website` and latest release branch backport/1.7.x Backport PR changes to `release/1.7.x` labels Jan 11, 2022
@thiskevinwang thiskevinwang merged commit b334116 into master Jan 11, 2022
@thiskevinwang thiskevinwang deleted the kevin/plugins branch January 11, 2022 15:26
thiskevinwang added a commit that referenced this pull request Jan 11, 2022
* feat: extract `/plugins`

Co-authored-by: Zachary Shilton <4624598+zchsh@users.noreply.github.com>
Co-authored-by: Adrien Delorme <azr@users.noreply.github.com>
thiskevinwang added a commit that referenced this pull request Jan 11, 2022
* feat: extract `/plugins`

Co-authored-by: Zachary Shilton <4624598+zchsh@users.noreply.github.com>
Co-authored-by: Adrien Delorme <azr@users.noreply.github.com>
thiskevinwang added a commit that referenced this pull request Jan 11, 2022
* feat: extract `/plugins`

Co-authored-by: Zachary Shilton <4624598+zchsh@users.noreply.github.com>
Co-authored-by: Adrien Delorme <azr@users.noreply.github.com>

Co-authored-by: Kevin Wang <kwangsan@gmail.com>
Co-authored-by: Zachary Shilton <4624598+zchsh@users.noreply.github.com>
Co-authored-by: Adrien Delorme <azr@users.noreply.github.com>
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/website Backport PR changes to `stable-website` and latest release branch backport/1.7.x Backport PR changes to `release/1.7.x`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants