Skip to content
This repository has been archived by the owner on Sep 23, 2024. It is now read-only.

Rename class in publisher, and report by using the CapWords convention #153

Merged
merged 7 commits into from
May 22, 2018

Conversation

danghai
Copy link
Contributor

@danghai danghai commented May 22, 2018

PR to rename class in publisher, and report by using the CapWords convention
Resolve #3

@coveralls
Copy link

coveralls commented May 22, 2018

Pull Request Test Coverage Report for Build 482

  • 11 of 15 (73.33%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 46.658%

Changes Missing Coverage Covered Lines Changed/Added Lines %
skt/publisher.py 3 4 75.0%
skt/reporter.py 4 7 57.14%
Totals Coverage Status
Change from base Build 463: 0.0%
Covered Lines: 684
Relevant Lines: 1466

💛 - Coveralls

Copy link
Contributor

@veruu veruu left a comment

Choose a reason for hiding this comment

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

Hi Hai! Code changes looks good, thanks for the pull! However, I do have a small request to change the docstrings which are talking about the classes directly too. I noticed the docstring for TestConsoleLog still uses the lower case class name, and same with TestPublisher class. Can you add these changes to the right commits?

@danghai danghai force-pushed the skt_class_naming branch from 568b52e to 443e787 Compare May 22, 2018 09:05
@danghai
Copy link
Contributor Author

danghai commented May 22, 2018

Hello @veruu . I have updated! Thanks :)

Copy link
Contributor

@veruu veruu left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for the fast action! @spbnick do you want to take a quick look?

@spbnick spbnick merged commit 81dcb8d into cki-project:master May 22, 2018
@spbnick
Copy link
Contributor

spbnick commented May 22, 2018

Looks good, thank you Hai, Veronika! Merged!

@danghai danghai deleted the skt_class_naming branch May 22, 2018 17:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants