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

[Documentation] Making Docs code eslint compliant #1778

Merged
merged 6 commits into from
Oct 20, 2015

Conversation

NogsMPLS
Copy link
Contributor

  • Adding dangling commas where needed to make docs code pass eslint as defined in .eslintrc file.
  • Removing 'module.export' and replacing with export default where applicable.
  • Moving contextTypes to a static within component where applicable
  • Changing single quoted props to double quote where applicable
  • Adding spaces to arrays where applicable.

@oliviertassinari
Copy link
Member

Look great!
Still, I think that we should wait the react v0.14 support before merging this.

@NogsMPLS
Copy link
Contributor Author

NogsMPLS commented Oct 1, 2015

Any reason why on the wait? All changes are just es6 and lint changes, non are react specific.

unless these changes conflict with docs in the react 0.14 branch?

in which case, would you rather i apply these changes to that branch instead?

@cgestes
Copy link
Contributor

cgestes commented Oct 1, 2015

On Thu, Oct 1, 2015 at 2:17 AM, Nathan Smith [email protected]
wrote:

Any reason why on the wait? All changes are just es6 and lint changes, non
are react specific.

unless these changes conflict with docs in the react 0.14 branch?

in which case, would you rather i apply these changes to that branch
instead?

on either side it's gonna be difficult to fix the conflict. The best is to
apply the changes after we merge, or to apply on master, and to rebase the
react branch on top of that. (maybe by apply the transform on both branch,
and then rebasing). If you want to use your git foo, this is a work for you
!


Reply to this email directly or view it on GitHub
#1778 (comment)
.

@oliviertassinari
Copy link
Member

@NogsMPLS Our work with react v0.14 is almost done. We had to prioritize it over this PR.
If you can rebase it, It would be awesome!

@NogsMPLS
Copy link
Contributor Author

okay, lemme see if that worked. had quite a few merge conflicts, going through code committed now

@oliviertassinari
Copy link
Member

@NogsMPLS Thanks! Can you make eslint lint the docs source folder too, so that we prevent regression? Should be here https://github.com/callemall/material-ui/blob/master/gulpfile.js#L5.

super();
static contextTypes = {
router: React.PropTypes.func,
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needed anymore? Haven't been keeping up with changes, but I saw some router changes and other files changing contextTypes

Copy link
Member

Choose a reason for hiding this comment

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

Yes, you can remove it.

@NogsMPLS
Copy link
Contributor Author

docs folder added to eslint gulp task. just looking at jsx/js files since teh docs folder has that .txt thing going on with it. passing eslint on my machine.

@@ -2,12 +2,14 @@ const React = require('react');
const ReactDOM = require('react-dom');
const { Styles } = require('material-ui');
const { Spacing } = Styles;
const { ThemeManager } = Styles;
const DefaultRawTheme = Styles.LightRawTheme;
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

eslint was throwing me an error later down the file because Thememanager and DefaultRawTheme weren't defined. looked at what the other files in docs were doing to handle it and slapped it in there.

Copy link
Member

Choose a reason for hiding this comment

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

@oliviertassinari
Copy link
Member

@shaurya947 I thinks that we can merge.

@shaurya947
Copy link
Contributor

@oliviertassinari but there's conflicts

@NogsMPLS
Copy link
Contributor Author

whaaat, there wasn't last night!!!!

ill rebase again i suppose

@NogsMPLS
Copy link
Contributor Author

Okay, rebased, things should be good again

shaurya947 added a commit that referenced this pull request Oct 20, 2015
[Documentation] Making Docs code eslint compliant
@shaurya947 shaurya947 merged commit a30b146 into mui:master Oct 20, 2015
@shaurya947
Copy link
Contributor

Thanks @NogsMPLS

@zannager zannager added the docs Improvements or additions to the documentation label Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants