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

Fix part of #3825: Rename Group 1 services to UpperCamelCase #3948

Merged
merged 3 commits into from
Oct 10, 2017
Merged

Fix part of #3825: Rename Group 1 services to UpperCamelCase #3948

merged 3 commits into from
Oct 10, 2017

Conversation

abarya
Copy link
Contributor

@abarya abarya commented Oct 8, 2017

No description provided.

@abarya
Copy link
Contributor Author

abarya commented Oct 8, 2017

Hi @shubha1593,
Please review.

@codecov-io
Copy link

codecov-io commented Oct 8, 2017 •
edited
Loading

Codecov Report

Merging #3948 into develop will not change coverage.
The diff coverage is 35.71%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3948   +/-   ##
========================================
  Coverage    45.91%   45.91%           
========================================
  Files          300      300           
  Lines        21137    21137           
  Branches      3318     3318           
========================================
  Hits          9706     9706           
  Misses       11431    11431
Impacted Files Coverage Δ
...es/exploration_player/ConversationSkinDirective.js 2.2% <0%> (ø) ⬆️
core/templates/dev/head/pages/library/Library.js 29.78% <0%> (ø) ⬆️
...emplates/dev/head/pages/preferences/Preferences.js 44.44% <0%> (ø) ⬆️
...lates/dev/head/pages/library/SearchBarDirective.js 1.14% <0%> (ø) ⬆️
...ages/library/ActivityTilesInfinityGridDirective.js 5% <0%> (ø) ⬆️
.../pages/exploration_player/StatsReportingService.js 4.44% <0%> (ø) ⬆️
...re/templates/dev/head/services/MessengerService.js 1.58% <100%> (ø)
core/templates/dev/head/services/SearchService.js 40.18% <100%> (ø)
core/templates/dev/head/services/UtilsService.js 100% <100%> (ø)
core/templates/dev/head/filters.js 64.35% <100%> (ø) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a481e7a...7bde4cd. Read the comment docs.

shubha1593
shubha1593 suggested changes Oct 9, 2017
View reviewed changes
Copy link
Contributor

@shubha1593 shubha1593 left a comment

Choose a reason for hiding this comment

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

Hi @abhishekarya286, thanks! Just two changes: could you rename 'searchServiceSpec.js' to 'SearchServiceSpec.js' and similarly change 'utilsServiceSpec.js' to 'UtilsServiceSpec.js' ?
Also notice, there are merge conflicts.

@abarya
Copy link
Contributor Author

abarya commented Oct 9, 2017

Hi @shubha1593, I have renamed the files and resolved the merge conflicts.
Please review.

@shubha1593
Copy link
Contributor

Hi @abhishekarya286, Travis check is failing. I guess you need to resolve the linting errors.

@abarya
Copy link
Contributor Author

abarya commented Oct 9, 2017

Hi @shubha1593, thanks for pointing out. I've removed the linting errors now.

shubha1593
shubha1593 approved these changes Oct 10, 2017
View reviewed changes
Copy link
Contributor

@shubha1593 shubha1593 left a comment

Choose a reason for hiding this comment

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

@abhishekarya286 Thanks! LGTM.

@shubha1593 shubha1593 changed the title Fix #3825: Rename Group 1 services to UpperCamelCase Fix part of #3825: Rename Group 1 services to UpperCamelCase Oct 10, 2017
@shubha1593 shubha1593 merged commit ab15c39 into oppia:develop Oct 10, 2017
@abarya
Copy link
Contributor Author

abarya commented Oct 10, 2017

Thanks @shubha1593 for merging. Also, I'm participating in hacktoberfest so I want to know whether this PR would be counted.

@seanlip
Copy link
Member

seanlip commented Oct 10, 2017

Hi @abhishekarya286, I think it will! :)

@abarya abarya deleted the UpperCamelCase branch October 11, 2017 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shubha1593 shubha1593 shubha1593 approved these changes

Assignees
No one assigned
Labels
PR: LGTM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@abarya @codecov-io @shubha1593 @seanlip