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

[WIP] engine: move static/ contents into public/ and update links #879

Closed
wants to merge 6 commits into from

Conversation

jorgeorpinel
Copy link
Contributor

@jorgeorpinel jorgeorpinel commented Dec 23, 2019

@shcheklein shcheklein temporarily deployed to dvc-landing-jorgeorpine-bvadoh December 23, 2019 09:32 Inactive
@jorgeorpinel jorgeorpinel changed the title engine: move static/ contents into public/ and shorten static links in contents engine: move static/ contents into public/ and shorten static asset links Dec 23, 2019
import glossary from '../../static/docs/glossary'
import glossary from '../../public/static/docs/glossary'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iAdramelk @shcheklein Isn't it a little funny to import a static asset from src code? I know we talked about this change but I didn't realize this would be the result 😋

Copy link
Contributor

Choose a reason for hiding this comment

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

In perfect world it, together with the md files, should be placed in the content folder in root directory and moved automatically to public folder at build time, but in next.js we would need to do it by hand and it's too bothersome.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK makes sense. I'll leave this review open for future ref.

src/utils/sidebar.js Outdated Show resolved Hide resolved
to complete previous commit's intended change
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorgeorpine-bvadoh December 23, 2019 09:42 Inactive
@jorgeorpinel jorgeorpinel changed the title engine: move static/ contents into public/ and shorten static asset links engine: move static/ contents into public/ and update links Dec 23, 2019
update all links from contents WITHOUT proper formatting (yet).
See https://nextjs.org/docs#static-file-serving-eg-images
@jorgeorpinel

This comment has been minimized.

@iAdramelk

This comment has been minimized.

@jorgeorpinel

This comment has been minimized.

@iAdramelk

This comment has been minimized.

@jorgeorpinel
Copy link
Contributor Author

Yep same. /doc shows the 404 page (with a 200 status code BTW). Other /doc/{valid} routes also show 404s (but with actual 404 code). The strange thing is that it works on https://dvc-landing-jorgeorpine-bvadoh.herokuapp.com/doc 😋 which is why I think it has to do both with my move from static to public, and with your changes in #872 – specifically https://github.com/iterative/dvc.org/pull/872/files#diff-78c12f5adc1848d13b1c6f07055d996eR110-R113 maybe. I'll look into it...

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorgeorpine-bvadoh December 23, 2019 23:45 Inactive
@jorgeorpinel

This comment has been minimized.

@jorgeorpinel jorgeorpinel marked this pull request as ready for review December 23, 2019 23:50
@jorgeorpinel

This comment has been minimized.

@jorgeorpinel jorgeorpinel changed the title engine: move static/ contents into public/ and update links [WIP] engine: move static/ contents into public/ and update links Dec 23, 2019
@jorgeorpinel

This comment has been minimized.

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorgeorpine-bvadoh December 24, 2019 02:30 Inactive
@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Dec 24, 2019

+ close #804 (adopt restyled.io)

Failed miserably. We installed Restyled on this repo but the check is just not coming up. 🤷‍♂ Removed the config file for now.

@jorgeorpinel
Copy link
Contributor Author

Hahaha. Now it works!

image

...

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-jorgeorpine-bvadoh December 24, 2019 02:38 Inactive
@jorgeorpinel
Copy link
Contributor Author

I added the .restyled.yaml config file again but Restyled thinks there's "No differences". Weird. I'll try to close and reopen the PR...

@shcheklein shcheklein temporarily deployed to dvc-landing-jorgeorpine-hlkorw December 24, 2019 02:42 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: website Area: website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lint: adopt restyled.io
3 participants