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

Add initial script for auto doc-gen #2128

Merged
merged 1 commit into from Aug 14, 2020

Conversation

ayushjainrksh
Copy link
Contributor

@ayushjainrksh ayushjainrksh commented Aug 5, 2020

Summary

The PR is part of an effort to merge the initial script created by @motiz88 . The project is a part of MLH fellowship program and involves automatic generation of the website docs from code comments and flow types as the end result.

To learn more about the project you can visit the project wiki:

Changes

  • Add experimental script to sync API docs from code
  • sync-api-docs: Support methods
  • [remove before merging] Sync some API docs
  • [remove before merging] Netlify fix
  • sync-api-docs: add docs extraction script
  • sync-api-docs: Improvements to method docs
  • sync-api-docs: update docgen fork to 0.0.1
  • [remove before merging] write extracted.json
  • sync-api-docs: pass filename into react-docgen
  • sync-api-docs: don't use PropTypes composition handler
    We have Flow types for this.
  • sync-api-docs: remove unused comment-parser
  • sync-api-docs: bump docgen fork to 0.0.2
  • [remove before merging] update Markdown files
  • Sync docs with upstream

Changelog

[Internal]

* Add experimental script to sync API docs from code

* sync-api-docs: Support methods

* [remove before merging] Sync some API docs

* [remove before merging] Netlify fix

* sync-api-docs: add docs extraction script

* sync-api-docs: Improvements to method docs

* sync-api-docs: update docgen fork to 0.0.1

* [remove before merging] write extracted.json

* sync-api-docs: pass filename into react-docgen

* sync-api-docs: don't use PropTypes composition handler

We have Flow types for this.

* sync-api-docs: remove unused comment-parser

* sync-api-docs: bump docgen fork to 0.0.2

* [remove before merging] update Markdown files

* Sync docs with upstream

Co-authored-by: Moti Zilberman <motiz88@gmail.com>
Co-authored-by: Aniket Kumar <aniketkumar049@gmail.com>
@react-native-bot
Copy link

Deploy preview for react-native ready!

Built with commit 5a03c82

https://deploy-preview-2128--react-native.netlify.app

Changes to docs/ are reflected in the next "master" version.

Thank you for your contributions.

How to ContributeDocumentation Sources

@@ -9,35 +9,25 @@
dependencies:
"@babel/highlight" "^7.0.0"

"@babel/code-frame@^7.10.4":
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's something wrong with the Yarn lockfile, because dependencies are downgraded. Is your branch up to date with the master?

@@ -41,17 +41,21 @@
"alex": "^8.0.0",
"docusaurus": "1.14.5",
"highlight.js": "^9.15.10",
"remarkable": "^2.0.0"
"remarkable": "^2.0.0",
"tokenize-comment": "^3.0.1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why tokenize-comment is located under dependencies instead of devDependencies? Did I miss something?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, this can be a dev dependency like the rest of the doc generation script's dependencies. I'm not sure there's a strong distinction though, given the way the website is deployed at the moment (e.g. does the alex linter need to be under dependencies?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

(e.g. does the alex linter need to be under dependencies?)

That's a good question, I'm going to check this and and based on the results I will adjust the #2140 PR. Alex has been added almost a year ago when I was not an active contributor.

Copy link
Collaborator

@Simek Simek left a comment

Choose a reason for hiding this comment

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

Can you explain how this PR is connected to the #1628 and #2129? Also merging this one will cause conflicts with #2129, what's the reason for the separate PRs?

I was trying to add commit with updated Prettier paths in package.json but my push was rejected. Please change the repository permissions or add new files path to the Prettier config.

@Simek Simek added the MLH Major League Hacking label Aug 13, 2020
@jevakallio
Copy link

jevakallio commented Aug 13, 2020

I was trying to add commit with updated Prettier paths in package.json but my push was rejected. Please change the repository permissions or add new files path to the Prettier config.

@Simek I've sent you an invitation to collaborate on the fork 👍

@motiz88 should be able to address questions about the dependencies.

The split between this and #2129 was intended so that it would be easier for @motiz88 to review the changes made by @ayushjainrksh and @ani4aniket over at #2129 without having to review his own changes in this PR. I think the original intent was to base #2129 off this one, so the diff over there would only show the additional commits.

To avoid conflicts, it may make sense to make any amendments at #2129, and close this as unnecessary.

@Simek
Copy link
Collaborator

Simek commented Aug 13, 2020

@jevakallio Thank you for the answers and the invite! 🙂

To avoid conflicts, it may make sense to make any amendments at #2129, and close this as unnecessary.

I like that idea, I'm going to repost my comments in the second PR and I will add the Prettier paths commit in there too.

And I'm leaving this PR as is, please close it after you perform the necessary changes to the other one.

@motiz88 motiz88 merged commit 5a03c82 into facebook:master Aug 14, 2020
@Simek
Copy link
Collaborator

Simek commented Aug 14, 2020

@motiz88 This PR should not be merged at all. It includes the dependencies downgrade 😞

Simek added a commit that referenced this pull request Aug 14, 2020
@motiz88
Copy link
Contributor

motiz88 commented Aug 14, 2020

To be clear, I didn't merge this directly - I merged #2129 which contains the same commits plus additional work and fixups (admittedly I should have probably squashed it, my bad). That is why the GitHub UI is showing this PR as merged too. Does #2129 have the correct state of the dependencies? If so, that's the state of master right now.

@Simek
Copy link
Collaborator

Simek commented Aug 14, 2020

Does #2129 have the correct state of the dependencies? If so, that's the state of master right now.

@motiz88 It have the corrected lock, but they way you have merged this together caused an issue: 6e41a04

gedeagas pushed a commit to gedeagas/react-native-website that referenced this pull request Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed MLH Major League Hacking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants