Skip to content

Conversation

@chalettu
Copy link
Contributor

Made it consistent across the project how classNames package is imported and used by all components

Fix #230

@coveralls
Copy link

Pull Request Test Coverage Report for Build 966

  • 70 of 71 (98.59%) changed or added relevant lines in 68 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 71.391%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/components/UtilizationBar/helpers.js 1 2 50.0%
Totals Coverage Status
Change from base Build 964: 0.0%
Covered Lines: 1244
Relevant Lines: 1574

💛 - Coveralls

@coveralls
Copy link

coveralls commented Mar 16, 2018

Pull Request Test Coverage Report for Build 992

  • 70 of 71 (98.59%) changed or added relevant lines in 68 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 71.577%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/components/UtilizationBar/helpers.js 1 2 50.0%
Totals Coverage Status
Change from base Build 988: 0.0%
Covered Lines: 1255
Relevant Lines: 1585

💛 - Coveralls

@priley86
Copy link
Member

priley86 commented Mar 19, 2018

greetings @chalettu and welcome to the pf-react community ✌️

this is very much appreciated! I think standards like this make the code much more readable and easier to follow for all of us (beginners especially).

the changes here look great! two more small more requests for this PR to close the issue:

  • can you add some brief note about classNames to the coding conventions section in CONTRIBUTING.md?
  • can you possibly rebase your commit just a tad... refactor(classNames): Updated how classNames package is referenced ... we use conventional-changelog convention... if you are ever stuck on this, npm run commit can help...

Made it consistent across the project how classNames package is imported and used by all components

Fix patternfly#230
@chalettu
Copy link
Contributor Author

@priley86 , I made the updates you requested. Can you take a look at this PR again? Thank you

Copy link
Member

@jeff-phillips-18 jeff-phillips-18 left a comment

Choose a reason for hiding this comment

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

Thanks @chalettu!

@jeff-phillips-18 jeff-phillips-18 merged commit 438849c into patternfly:master Mar 21, 2018
@jgiardino jgiardino removed the review label Mar 21, 2018
@chalettu chalettu deleted the classNames branch April 6, 2018 14:14
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.

5 participants