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

[AC-441] Convert Hardcoded Strings to String Resources #627

Merged
merged 1 commit into from
Dec 17, 2019
Merged

[AC-441] Convert Hardcoded Strings to String Resources #627

merged 1 commit into from
Dec 17, 2019

Conversation

ribhavsharma
Copy link
Contributor

@ribhavsharma ribhavsharma commented Dec 2, 2019

Description of what I changed

Converted all hardcoded strings to String Resources, so that the app can have support for multiple languages in the future.

Task that I worked on

GCI Task: Android Client: Convert Hardcoded Strings to String Resources

Issue that I worked on

AC-441

Checklist: I completed these to help reviewers :)

  • My pull request only contains ONE single commit
    (the number above, next to the 'Commits' tab is 1).
  • I have added tests to cover my changes. (If you refactored
    existing code that was well tested you do not have to add tests)
  • All new and existing tests passed.
  • My pull request is based on the latest changes of the master branch.

@codecov-io
Copy link

codecov-io commented Dec 2, 2019

Codecov Report

Merging #627 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #627   +/-   ##
=======================================
  Coverage   14.29%   14.29%           
=======================================
  Files         202      202           
  Lines        9220     9220           
  Branches      791      791           
=======================================
  Hits         1318     1318           
  Misses       7821     7821           
  Partials       81       81

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 7a912ec...156ce52. Read the comment docs.

Copy link
Contributor

@prathamesh-mutkure prathamesh-mutkure left a comment

Choose a reason for hiding this comment

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

Hii @ribhavsharma

I have reviewed your code, looks good but can you please work on suggested changes? I've added comments on the same.

I'm not a mentor so wait for them to review it and have their say!

Copy link
Contributor

@vansha10 vansha10 left a comment

Choose a reason for hiding this comment

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

@ribhavsharma Please follow the Pull Request Guidelines.
You need to include the JIRA ticket number in the Pull Request Name, and also have a link to it in the description.
Also, please squash your commits into a single commit.

@ribhavsharma
Copy link
Contributor Author

ribhavsharma commented Dec 5, 2019

@ribhavsharma Please follow the Pull Request Guidelines.
You need to include the JIRA ticket number in the Pull Request Name, and also have a link to it in the description.
Also, please squash your commits into a single commit.

@vansha10 Kindly check out the PR now.

@vansha10
Copy link
Contributor

vansha10 commented Dec 6, 2019

Good work @ribhavsharma.
Just one more thing, you need to include the JIRA ticket number in the Pull Request Title. Please replace
[GCI '19] with AC 441.

@ribhavsharma ribhavsharma changed the title [GCI '19] Convert Hardcoded Strings to String Resources [AC-441] Convert Hardcoded Strings to String Resources Dec 6, 2019
@vansha10
Copy link
Contributor

vansha10 commented Dec 6, 2019

Looks good @ribhavsharma.
@f4ww4z Can you have a look at this?

@f4ww4z f4ww4z merged commit 1d4fc3d into openmrs:master Dec 17, 2019
@f4ww4z
Copy link
Collaborator

f4ww4z commented Dec 17, 2019

Looks @ribhavsharma ! Sorry for the delay.

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