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

breaking(v2): editUrl should point to website instead of docsDir #1958

Merged
merged 1 commit into from
Nov 11, 2019

Conversation

endiliey
Copy link
Contributor

Motivation

In v1, editUrl is pointing to docsDir and its always pointing to next for the edit.

Try https://docusaurus.io/docs/en/1.13.0/installation, notice the edit is for next docs only.

This breaking changes makes people moving docs folder around easier.

For example, if I renamed docs folder from docs to document. Before this PR i have to change the docs plugin path options from docs to document AND I HAVE to update my editUrl from https://github.com/facebook/docusaurus/edit/master/website/docs/ to https://github.com/facebook/docusaurus/edit/master/website/document/ as well. Double work.
After this PR, I only need to do it once

It supports versioned_docs and translated_docs editUrl better in future

Since we can always resolve it based from website dir. Since we already have lot of aliasing through @site, it makes more sense to point it to website instead of docsDir.

Have you read the Contributing Guidelines on pull requests?

yes

Test Plan

  • Updated test passes
  • Netlify editUrl still works

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@endiliey endiliey added the pr: breaking change Existing sites may not build successfully in the new version. Description contains more details. label Nov 11, 2019
@endiliey endiliey requested a review from yangshun as a code owner November 11, 2019 11:51
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 11, 2019
@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-2 ready!

Built with commit 3945290

https://deploy-preview-1958--docusaurus-2.netlify.com

@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-preview ready!

Built with commit 3945290

https://deploy-preview-1958--docusaurus-preview.netlify.com

Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

It supports versioned_docs and translated_docs editUrl better in future

Good foresight. My only concern is that many users won't read the release notes and their docs edit button points to a broken URL after upgrading. I wonder if there's a way to help warn them about it, possibly just by searching for website/docs in the editUrl. This is also helpful for users porting from v1, not just v2 alpha dogfooders.

@endiliey
Copy link
Contributor Author

I'm also not sure on the best approach. For now I've tried to minimize the damage by editing the docs on migration.

@endiliey endiliey added the status: don't merge yet This PR is completed, but we are still waiting for other things to be ready. label Nov 11, 2019
@endiliey
Copy link
Contributor Author

Don't merge since it includes docs change. Will only merge right before release new alpha

@endiliey endiliey merged commit 6a3efd5 into master Nov 11, 2019
@endiliey endiliey deleted the endi/editurl branch November 11, 2019 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: breaking change Existing sites may not build successfully in the new version. Description contains more details. status: don't merge yet This PR is completed, but we are still waiting for other things to be ready.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants