Skip to content

Conversation

@fseto
Copy link

@fseto fseto commented Jan 7, 2017

Copy link
Contributor

@diegocarloslima diegocarloslima left a comment

Choose a reason for hiding this comment

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

Thanks for you contribution. It looks good. I left a comment of a small change that should be made :)

<resources>

<string name="app_name">Adblock Plus for Samsung Internet</string>
<string name="app_name">Adblock Plus</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

We still want to keep the current naming for the MainPreferences activity. Given that, instead of modifying the app_name string, we need to add a new item <string name="launcher_name">Adblock Plus</string> and use it in the application label on AndroidManifest.xml

Copy link
Author

Choose a reason for hiding this comment

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

That is a good point. I will make the changes.

Copy link
Contributor

@diegocarloslima diegocarloslima left a comment

Choose a reason for hiding this comment

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

I left a comment about changes required in AndroidManifest.xml


<string name="app_name">Adblock Plus</string>
<string name="app_name">Adblock Plus for Samsung Internet</string>
<string name="launcher_name">Adblock Plus</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

Just one more thing, you need to use this launcher_name string in AndroidManifest. Under the <application> tag, change the label to android:label="@string/launcher_name"

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your continued guidance Diego! I have updated AndroidManifest.

@4ntoine
Copy link

4ntoine commented Jan 27, 2017

LGTM after Diego's changes taken into account

@diegocarloslima
Copy link
Contributor

Hey @fseto, before we push your patch, we need you to sign the contributor agreement. Let me know if you have any questions. Cheers!

abpbot pushed a commit that referenced this pull request Nov 15, 2018
abpbot pushed a commit that referenced this pull request Nov 16, 2018
Issue #1 - Migrate to default Android project structure

See merge request eyeo/adblockplus/adblockplussbrowser!4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants