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.

Sign up for GitHub

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

Jump to bottom

[docs] Generate docs for components #2465

Merged
merged 19 commits into from
Oct 22, 2021
Merged

[docs] Generate docs for components #2465

merged 19 commits into from
Oct 22, 2021

Conversation

m4theushw
Copy link
Member

@m4theushw m4theushw commented Aug 28, 2021 •
edited
Loading

@m4theushw m4theushw force-pushed the add-docs-for-components branch from 903a4f8 to e7524bb Compare August 28, 2021 01:24
@m4theushw m4theushw added the docs Improvements or additions to the documentation label Aug 28, 2021
@flaviendelangle flaviendelangle mentioned this pull request Sep 6, 2021
Merged
@m4theushw m4theushw mentioned this pull request Sep 7, 2021
Merged
@m4theushw m4theushw force-pushed the add-docs-for-components branch from e7524bb to e316c98 Compare September 16, 2021 02:06
@m4theushw m4theushw force-pushed the add-docs-for-components branch from e316c98 to 0be463e Compare September 16, 2021 02:19
m4theushw
m4theushw commented Sep 16, 2021
View reviewed changes
docs/scripts/buildApi.ts
const indexPath = path.resolve(__dirname, '../../packages/grid/_modules_/index.ts');
const program = ttp.createTSProgram([...componentsToGenerateDocs, indexPath], tsconfig);

// Uncomment below to generate documentation for all exported components
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 prefer to only document now DataGrid and DataGridPro. There're a few small components we could merge together into bigger and meaninful components. Not documenting them allows us to change them without declaring breaking changes.

package.json
"@emotion/cache": "^11.4.0",
"@emotion/react": "^11.4.1",
"@emotion/styled": "^11.3.0",
"@eps1lon/enzyme-adapter-react-17": "^0.1.0",
"@material-ui/monorepo": "https://github.com/mui-org/material-ui.git#next",
"@material-ui/monorepo": "https://github.com/m4theushw/material-ui.git#master",
Copy link
Member Author

Choose a reason for hiding this comment

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

Once we're okay with the shape of this PR, I'll open another one in the core with the changes from my fork. Basically what I did was to move code from the core's script to separate files to reuse here. Here's the diff: mui/material-ui@master...m4theushw:master

@m4theushw m4theushw marked this pull request as ready for review September 16, 2021 03:11
@flaviendelangle
Copy link
Member

flaviendelangle commented Sep 16, 2021 •
edited
Loading

Very nice !

I suppose some of those remarks are based on the core code, not your PR

image
image

What is the type inline behavior ?
For the logger is would prefer to have the type name (if possible with a link to a page describing it)

image

I think the following would be more readable

image

@m4theushw m4theushw added this to the Sprint 22 milestone Sep 17, 2021
@m4theushw m4theushw mentioned this pull request Sep 20, 2021
Merged
oliviertassinari
oliviertassinari reviewed Sep 20, 2021
View reviewed changes
yarn.lock
@@ -2138,9 +2185,9 @@
npmlog "^4.1.2"
write-file-atomic "^3.0.3"

"@material-ui/monorepo@https://github.com/mui-org/material-ui.git#next":
"@material-ui/monorepo@https://github.com/m4theushw/material-ui.git#master":
Copy link
Member

Choose a reason for hiding this comment

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

Weird (m4theushw)

Copy link
Member Author

Choose a reason for hiding this comment

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

#2465 (comment)

@m4theushw m4theushw mentioned this pull request Sep 27, 2021
Merged
flaviendelangle
flaviendelangle reviewed Sep 28, 2021
View reviewed changes
babel.config.js Show resolved Hide resolved
docs/pages/api-docs/data-grid/data-grid.js

Page.getInitialProps = () => {
const req = require.context('docsx/translations/api-docs/data-grid', false, /data-grid.*.json$/);
const descriptions = mapApiPageTranslations(req);
Copy link
Member

Choose a reason for hiding this comment

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

I don't know how heavy is the JSON to MD processing
But we may want to cache these pages at some point (same of the core).

Not blocking for this PR ofc

docs/scripts/buildApi.ts Show resolved Hide resolved
docs/scripts/buildApi.ts Outdated Show resolved Hide resolved
packages/grid/_modules_/grid/models/gridSlotsComponent.ts Outdated Show resolved Hide resolved
@flaviendelangle flaviendelangle mentioned this pull request Oct 1, 2021
Closed
@m4theushw m4theushw mentioned this pull request Oct 1, 2021
Closed
2 tasks
@github-actions
Copy link

github-actions bot commented Oct 1, 2021

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 1, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 1, 2021
m4theushw and others added 2 commits October 4, 2021 10:44
@m4theushw m4theushw mentioned this pull request Oct 4, 2021
Merged
1 task
@DanailH
Copy link
Member

DanailH commented Oct 5, 2021

Should we proceed with setting the correct repo in the yarn.lock and merging this?

@flaviendelangle
Copy link
Member

Let's go, it would be nice to have this one on this week release

@m4theushw
Copy link
Member Author

We need to wait for mui/material-ui#28828 to be merged.

@flaviendelangle
Copy link
Member

Right, the timing may be to short... next one then 😛

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 8, 2021
@github-actions
Copy link

github-actions bot commented Oct 8, 2021

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 22, 2021
@m4theushw
Copy link
Member Author

I'll merge this keeping my fork. We need the auto-generated docs ASAP. Once mui/material-ui#28828 is merged I'll revert to use the correct monorepo. I had to change the cache key used by the CircleCI to force the cache invalidation. After rebasing mui/material-ui#28828, it broke because it was trying to merge when git pulling.

@m4theushw m4theushw merged commit aeec7d7 into mui:next Oct 22, 2021
@m4theushw m4theushw deleted the add-docs-for-components branch October 22, 2021 18:21
This was referenced Oct 27, 2021
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliviertassinari oliviertassinari oliviertassinari left review comments

@DanailH DanailH DanailH approved these changes

@flaviendelangle flaviendelangle flaviendelangle approved these changes

Assignees

@m4theushw m4theushw

Labels
docs Improvements or additions to the documentation
Projects
None yet
Milestone
v5 stable version
Development

Successfully merging this pull request may close these issues.
4 participants
@m4theushw @flaviendelangle @DanailH @oliviertassinari