-
Notifications
You must be signed in to change notification settings - Fork 394
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
[WIP] Gatsby migration #1025
[WIP] Gatsby migration #1025
Conversation
@fabiosantoscode could start reviewing this? ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lookin' good! Just a few comments. Also for the API/redirects to work locally and in production, have a look at: https://github.com/iterative/blog/pull/122/files. You just have to make the API and the redirects into express middleware, then mount it into an express server for production, and in the gatsby development server using gatsby-config.
hitType: 'exception', | ||
exDescription, | ||
exFatal | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside: We can probably use something like sentry or crashlytics here. If we end up implementing it in another iterative property we can plop it here as well at no additional cost.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This, or maybe switch to GTM and send it from there.
This is a gargantuan amount of work, good luck @iAdramelk :) |
c92f450
to
3f56e6b
Compare
@fabiosantoscode I added AIP and reedirects middlewares + simple express server, can you please review them and advice on additional headers and other stuff that I maybe should add here? |
@iAdramelk please rebase/merge, and let's check why CI is failing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- page titles do not update
- edit on Github does not work
- getting / related redirects - should be fixed probably the same way we fixed it for blog
- NEXT on https://dvc-landing-gatsby-migr-abefpq.herokuapp.com/doc/command-reference/version does not work
8efd044
to
2121a2e
Compare
…sby's API to not make full rerender of page on navigation. It fixes transition in doc's sidebar
2121a2e
to
d22140b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow up on my previews review on #1025 (comment)
- The DOC link still seems to be hardcoded to /doc/get-started. What if we change the first doc in the future, e.g. to /doc/install, will the link change automatically?
/doc path redirects to /doc/get-started. Again, what if we want to change the first doc to something else? Would we have to change the redirects file every time?Moving to nav: fix problem with non-existing pages in sidebar #1060- And if we have to do this (instead of just loading the first doc and keeping the /doc path) maybe change the /docs and /documentation redirects that currently go to /doc, so they go directly to the first doc? (Also keeping /doc -> /doc/get-started some time for legacy 301 support)
- How can we test the subdomain redirects? (www, man, error, code/data/remote)
- URL path redirects e.g. /documentation/command-reference do not add the required trailing slash e.g. -> /doc/command-reference, which causes a 2nd redirect to the final location: /doc/command-reference/
@jorgeorpinel there is a ticket to handle all of this after the merge - #1060 - please chime in here.
we will deploy and test them
will be handled when we merge dvc.org with blog the way I understand this cc @fabiosantoscode @iAdramelk @pavelgrinchenko |
It's actually not hardcoded now. It resolves to the first item with the source in the |
I'm seeing it hardcoded in redirects-list.json @iAdramelk |
It's hardcoded in redirects for now (to redirect users who hit urls from search engines or bookmarks), but actual links in html and site header/footer are not hardcoded. They are rosolved using sidebar.json. But you are right, it's not an ideal situation and it can break if we change root sidebar.json right now without changing redirects. |
ToDo:
_document.js
content tohtml.js
andSEO
files.LocalLink
to usegatsby-link
._app.js
to gatsby plugin.styled-components
SSR in gatsby.source: false
menu items work (update logic fornormalizedSidebar
generation)headings
extraction to backenddvc
from blogusage
vim
details
andabbr