Skip to content

Set up a minimal D2 site #2

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

Closed
wants to merge 25 commits into from
Closed

Set up a minimal D2 site #2

wants to merge 25 commits into from

Conversation

teikjun
Copy link
Member

@teikjun teikjun commented Aug 28, 2020

Summary

This PR sets up a minimal D2 site.

With reference to docs/v2-migration branch:

  • this PR cherry-picks up until the commit 6d1057f: fix errors in docs.
  • this PR excludes commits for CSS file migration (a663bc7 and 684e70b) to keep the PR minimal.

Log

  • Run migration CLI
  • Remove unused versions
  • remove from /versioned_docs, /versioned_sidebar, versions.json, docusaurus.config.js
  • Fix build errors from all versioned_docs and current docs
  • close all unclosed tags, remove invalid tags
  • for any invalid JSX outside codeblocks, enclose them in codeblocks
  • use MDX compatible syntax: change class to className, change style="..." to style={{...}}.
  • Fix other build errors
  • Fix url-loader issue
  • Fix errors in blog
  • Migrate navbar and footer items

Suggestions for next PRs

  • migrate CSS files (remember to include a663bc7 and 684e70b)
  • migrate pages
  • replace snack player remarkable plugin with remark plugin.
  • add tabs

The next PRs should be much smaller

Remarks

The bug that results in disappearing doc content is already present at this stage.

@teikjun
Copy link
Member Author

teikjun commented Aug 28, 2020

PR is now ready for review! :)

@teikjun teikjun requested review from darshkpatel and slorber August 28, 2020 12:32
Copy link

@slorber slorber left a comment

Choose a reason for hiding this comment

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

Thanks @teikjun

Can you fix the netlify warning and ensure https is used on gravatar URLs?
https://github.com/react-native-website-migration/react-native-website/pull/2/checks?check_run_id=1041310406

Also I've noticed that some docs are different from upstream. I guess you cherry-picked existing commits but the original docs did change in the meantime right?

I think it's not a big deal for how, as we'll need to integrate the latest docs changes when we are ready (and maybe include a few more versions), we'll need to re-run the docs migration anyway.


```jsx
static getRunnable(appKey)
static registerComponent(appKey, componentProvider, section?)
Copy link

Choose a reason for hiding this comment

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

Was wondering why this is changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I may have resolved the merge conflict incorrectly. I will revert fix errors in docs and try again

Copy link
Member Author

Choose a reason for hiding this comment

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

It's an issue with the original commit (not the merging), we'll check the other docs too

@darshkpatel
Copy link
Member

darshkpatel commented Aug 28, 2020

Screenshot 2020-08-28 at 7 01 28 PM

I believe you've made some mistake while reverting. The merge conflict headers are present

@teikjun
Copy link
Member Author

teikjun commented Aug 28, 2020

yup, I've made the changes already, will push soon (edit: done! it's in resolve merge conflict, above your comment)

| ---- | ---- | -------- | ----------- |


<<<<<<< HEAD | keyValuePairs | `Array<Array<string>>` | Yes | Array of key-value array for the items to set. | ======= | keyValuePairs | `Array<Array <string>>` | Yes | Array of key-value array for the items to set. |
Copy link
Member

Choose a reason for hiding this comment

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

Merge headers present

| ---- | ---- | -------- | ----------- |


<<<<<<< HEAD | keyValuePairs | `Array<Array<string>>` | Yes | Array of key-value array for the items to merge. | ======= | keyValuePairs | `Array<Array <string>>` | Yes | Array of key-value array for the items to merge. |
Copy link
Member

Choose a reason for hiding this comment

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

Merge headers present

@darshkpatel
Copy link
Member

darshkpatel commented Aug 28, 2020

@teikjun Also, in asyncstorage.md ( all versions ) you should ideally merge the current change instead of the incoming while cherry-picking since the upstream files don't have a space while our versions do, less files with changes if we ignore the space. it's trivial

Copy link
Member

@darshkpatel darshkpatel left a comment

Choose a reason for hiding this comment

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

Still some unresolved merge headers present
#2 (comment)
#2 (comment)

@teikjun
Copy link
Member Author

teikjun commented Aug 28, 2020

@darshkpatel thanks, I'll make the changes!
btw, could u check out MLH-Fellowship@6d1057f? (related to slorber's comment)
It replaces a lot of the original docs content for some reason. We should check if this is a common issue in other docs.

@darshkpatel
Copy link
Member

@darshkpatel thanks, I'll make the changes!
btw, could u check out MLH-Fellowship@6d1057f? (related to slorber's comment)
It replaces a lot of the original docs content for some reason. We should check if this is a common issue in other docs.

Its because commits there were referenced from an old master, the master has changed since then.
This will continue happening until the last moment. So we shouldn't waste a lot of time doing that since we'd as such have to redo docs migration once before merging

@darshkpatel
Copy link
Member

darshkpatel commented Aug 28, 2020

we also shouldn't be merging PRs to master since the bot will reset all the changes

@teikjun
Copy link
Member Author

teikjun commented Aug 28, 2020

made the requested changes, and opened a PR against main branch instead: #3

@teikjun teikjun closed this Aug 28, 2020
@teikjun teikjun mentioned this pull request Aug 28, 2020
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants