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
Feature/Moving main url
out of strings.xml
#27
Feature/Moving main url
out of strings.xml
#27
Conversation
release { | ||
minifyEnabled false | ||
proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro' | ||
resValue("string", "baseUrl", "\"https://dev.to\"") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is resValue
needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The resValue
has been added so that when in future the baseUrl
needs to be referred from resource files, it is already available to do so.
release { | ||
minifyEnabled false | ||
proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro' | ||
resValue("string", "baseUrl", "\"https://dev.to\"") | ||
buildConfigField("String", "baseUrl", "\"https://dev.to\"") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be made cleaner with:
buildConfigField("String", "baseUrl", "\"https://dev.to\"") | |
buildConfigField("String", "baseUrl", '"https://dev.to"') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@Glennmen thanks for your input here. Those are valid points but since there are no code here that's a deal breaker I'm going to merge it first. You are welcome to submit follow-up PR though!
@Glennmen @maestromac if the suggested changes are required then I can submit another PR for those, but I do suggest that we keep the |
What type of PR is this? (check all applicable)
Description
As per the discussion in issue #20 I have moved the main-url from string-resource to a build-config field in
build.gradle
.Related Tickets & Documents
Issue #20