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

chore(generator): update generator to support typescript files #1143

Merged
merged 1 commit into from
Jan 14, 2019

Conversation

ibolton336
Copy link
Member

@ibolton336 ibolton336 commented Jan 7, 2019

What: fix #1045

Additional issues:

@patternfly-build
Copy link
Contributor

PatternFly-React preview: https://1143-pr-patternfly-react-patternfly.surge.sh

@coveralls
Copy link

coveralls commented Jan 7, 2019

Pull Request Test Coverage Report for Build 3857

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 81.234%

Totals Coverage Status
Change from base Build 3852: 0.0%
Covered Lines: 4366
Relevant Lines: 5071

💛 - Coveralls

Copy link
Member

@dgutride dgutride left a comment

Choose a reason for hiding this comment

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

Is yarn.lock necessary in this change? I don't see any package changes.

className ?: string;
}

const {{componentName}}: React.SFC <{{componentName}}Props> = (props) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

I actually just saw today on another project that React.SFC will be replaced by React.FunctionComponent in the future. Might be good to switch to that now. Also you might want to remove the space between SFC and < (React.FunctionComponent<{{componentName}}Props>) prettier will likely remove it after the fact, but it would be better to avoid needing the cleanup step.

Copy link
Member

Choose a reason for hiding this comment

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

Looks good to me with the space removal - we should add an issue and make sure the FunctionComponent change above is good and make that change separately (to get this in and be consistent).

Copy link
Member Author

Choose a reason for hiding this comment

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

@dgutride updated the PR to remove the spaces.

dgutride
dgutride previously approved these changes Jan 9, 2019
className ?: string;
}

const {{componentName}}: React.SFC <{{componentName}}Props> = (props) => (
Copy link
Member

Choose a reason for hiding this comment

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

Looks good to me with the space removal - we should add an issue and make sure the FunctionComponent change above is good and make that change separately (to get this in and be consistent).

@ibolton336
Copy link
Member Author

Added an issue for the FunctionComponent changes #1149

@priley86
Copy link
Member

priley86 commented Jan 9, 2019

looks good to me @ibolton336 👍

Copy link
Contributor

@tlabaj tlabaj left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update PF4 component generator to default to typescript
8 participants