Issue 29348393: Issue 4259 - Change libadblockplus-android description in README (Closed)
|
Created:
July 22, 2016, 12:37 p.m. by anton Modified:
March 6, 2017, 9:12 a.m. CC:
vicky, René Jeschke Visibility:
Public. |
DescriptionIssue 4259 - Change libadblockplus-android description in README
Patch Set 1 #
Total comments: 4
Patch Set 2 : minor case changes #
Total comments: 1
Patch Set 3 : Added a brief description for all the modules. Removed duplicated section. #
Total comments: 2
Patch Set 4 : Removed 'An' #
Total comments: 12
Patch Set 5 : after felix's suggestions #
Total comments: 1
MessagesTotal messages: 13
https://codereview.adblockplus.org/29348393/diff/29348394/README.md File README.md (right): https://codereview.adblockplus.org/29348393/diff/29348394/README.md#newcode5 README.md:5: functionality in Java/Android. The choice of the best description is a bit subjective, but I would suggest changing to 'An Android library project that makes easier to use Adblock Plus core functionality in Java/Android.'
https://codereview.adblockplus.org/29348393/diff/29348394/README.md File README.md (right): https://codereview.adblockplus.org/29348393/diff/29348394/README.md#newcode5 README.md:5: functionality in Java/Android. On 2016/09/08 15:33:51, diegocarloslima wrote: > The choice of the best description is a bit subjective, but I would suggest > changing to 'An Android library project that makes easier to use Adblock Plus > core functionality in Java/Android.' Easier means that there is another way (probably more difficult or just another). One could create C++ android app and abp C++ classes directly without these "JNI-glue" classes but i'd say it's for hardcore developers who love pain. One could create completely JS app and JS abp code but it's also very rare use case. So i'd say using this library is only way to use abp core functionality.
https://codereview.adblockplus.org/29348393/diff/29348394/README.md File README.md (right): https://codereview.adblockplus.org/29348393/diff/29348394/README.md#newcode5 README.md:5: functionality in Java/Android. On 2016/09/09 06:48:18, anton wrote: > On 2016/09/08 15:33:51, diegocarloslima wrote: > > The choice of the best description is a bit subjective, but I would suggest > > changing to 'An Android library project that makes easier to use Adblock Plus > > core functionality in Java/Android.' > > Easier means that there is another way (probably more difficult or just > another). One could create C++ android app and abp C++ classes directly without > these "JNI-glue" classes but i'd say it's for hardcore developers who love pain. > One could create completely JS app and JS abp code but it's also very rare use > case. > > So i'd say using this library is only way to use abp core functionality. OK, got your point there, so I would just suggest to change 'use adblockplus core...' to 'use Adblock Plus core...'
https://codereview.adblockplus.org/29348393/diff/29348394/README.md File README.md (right): https://codereview.adblockplus.org/29348393/diff/29348394/README.md#newcode5 README.md:5: functionality in Java/Android. On 2016/11/03 10:48:45, diegocarloslima wrote: > On 2016/09/09 06:48:18, anton wrote: > > On 2016/09/08 15:33:51, diegocarloslima wrote: > > > The choice of the best description is a bit subjective, but I would suggest > > > changing to 'An Android library project that makes easier to use Adblock > Plus > > > core functionality in Java/Android.' > > > > Easier means that there is another way (probably more difficult or just > > another). One could create C++ android app and abp C++ classes directly > without > > these "JNI-glue" classes but i'd say it's for hardcore developers who love > pain. > > One could create completely JS app and JS abp code but it's also very rare use > > case. > > > > So i'd say using this library is only way to use abp core functionality. > > OK, got your point there, so I would just suggest to change 'use adblockplus > core...' to 'use Adblock Plus core...' Acknowledged. https://codereview.adblockplus.org/29348393/diff/29366721/README.md File README.md (right): https://codereview.adblockplus.org/29348393/diff/29366721/README.md#newcode4 README.md:4: An Android library project that provides an opportunity to use Adblock Plus core Now using like in https://hg.adblockplus.org/adblockpluscore/file/tip/README.md#l1
On 2016/12/02 06:10:03, anton wrote: > https://codereview.adblockplus.org/29348393/diff/29348394/README.md > File README.md (right): > > https://codereview.adblockplus.org/29348393/diff/29348394/README.md#newcode5 > README.md:5: functionality in Java/Android. > On 2016/11/03 10:48:45, diegocarloslima wrote: > > On 2016/09/09 06:48:18, anton wrote: > > > On 2016/09/08 15:33:51, diegocarloslima wrote: > > > > The choice of the best description is a bit subjective, but I would > suggest > > > > changing to 'An Android library project that makes easier to use Adblock > > Plus > > > > core functionality in Java/Android.' > > > > > > Easier means that there is another way (probably more difficult or just > > > another). One could create C++ android app and abp C++ classes directly > > without > > > these "JNI-glue" classes but i'd say it's for hardcore developers who love > > pain. > > > One could create completely JS app and JS abp code but it's also very rare > use > > > case. > > > > > > So i'd say using this library is only way to use abp core functionality. > > > > OK, got your point there, so I would just suggest to change 'use adblockplus > > core...' to 'use Adblock Plus core...' > > Acknowledged. > > https://codereview.adblockplus.org/29348393/diff/29366721/README.md > File README.md (right): > > https://codereview.adblockplus.org/29348393/diff/29366721/README.md#newcode4 > README.md:4: An Android library project that provides an opportunity to use > Adblock Plus core > Now using like in > https://hg.adblockplus.org/adblockpluscore/file/tip/README.md#l1 Is this issue still valid? The README file has been updated since then.
On 2017/02/03 18:13:22, diegocarloslima wrote: > On 2016/12/02 06:10:03, anton wrote: > > https://codereview.adblockplus.org/29348393/diff/29348394/README.md > > File README.md (right): > > > > https://codereview.adblockplus.org/29348393/diff/29348394/README.md#newcode5 > > README.md:5: functionality in Java/Android. > > On 2016/11/03 10:48:45, diegocarloslima wrote: > > > On 2016/09/09 06:48:18, anton wrote: > > > > On 2016/09/08 15:33:51, diegocarloslima wrote: > > > > > The choice of the best description is a bit subjective, but I would > > suggest > > > > > changing to 'An Android library project that makes easier to use Adblock > > > Plus > > > > > core functionality in Java/Android.' > > > > > > > > Easier means that there is another way (probably more difficult or just > > > > another). One could create C++ android app and abp C++ classes directly > > > without > > > > these "JNI-glue" classes but i'd say it's for hardcore developers who love > > > pain. > > > > One could create completely JS app and JS abp code but it's also very rare > > use > > > > case. > > > > > > > > So i'd say using this library is only way to use abp core functionality. > > > > > > OK, got your point there, so I would just suggest to change 'use adblockplus > > > core...' to 'use Adblock Plus core...' > > > > Acknowledged. > > > > https://codereview.adblockplus.org/29348393/diff/29366721/README.md > > File README.md (right): > > > > https://codereview.adblockplus.org/29348393/diff/29366721/README.md#newcode4 > > README.md:4: An Android library project that provides an opportunity to use > > Adblock Plus core > > Now using like in > > https://hg.adblockplus.org/adblockpluscore/file/tip/README.md#l1 > > Is this issue still valid? The README file has been updated since then. Updated according to the latest changes (see patch set #3)
Looks good, with one nit https://codereview.adblockplus.org/29348393/diff/29374623/README.md File README.md (right): https://codereview.adblockplus.org/29348393/diff/29374623/README.md#newcode88 README.md:88: An Android tests for the Library. The indefinite article 'An' can only be used for a singular subject. Given that, it should be removed.
https://codereview.adblockplus.org/29348393/diff/29374623/README.md File README.md (right): https://codereview.adblockplus.org/29348393/diff/29374623/README.md#newcode88 README.md:88: An Android tests for the Library. On 2017/02/06 20:35:33, diegocarloslima wrote: > The indefinite article 'An' can only be used for a singular subject. Given that, > it should be removed. Acknowledged.
On 2017/02/07 05:29:53, anton wrote: > https://codereview.adblockplus.org/29348393/diff/29374623/README.md > File README.md (right): > > https://codereview.adblockplus.org/29348393/diff/29374623/README.md#newcode88 > README.md:88: An Android tests for the Library. > On 2017/02/06 20:35:33, diegocarloslima wrote: > > The indefinite article 'An' can only be used for a singular subject. Given > that, > > it should be removed. > > Acknowledged. LGTM
https://codereview.adblockplus.org/29348393/diff/29374661/README.md File README.md (right): https://codereview.adblockplus.org/29348393/diff/29374661/README.md#newcode1 README.md:1: Adblock Plus for Android That's a bit confusing since we have a product called "Adblock Plus for Android", hm? I think "Adblock Plus Library for Android" is fine, since it's an Android library project. If you don't like that, how about "Libadblockplus for Android"? https://codereview.adblockplus.org/29348393/diff/29374661/README.md#newcode15 README.md:15: An Android library that provides an opportunity to use Adblock Plus core functionality Seems we could shorten this a bit: "An Android library that provides the core functionality of Adblock Plus. You can find it in the 'libadblockplus-android' directory." https://codereview.adblockplus.org/29348393/diff/29374661/README.md#newcode88 README.md:88: Android tests for the Library. Same here: "Android tests for the library. You can find them in the 'libadblockplus-android-tests' directory." https://codereview.adblockplus.org/29348393/diff/29374661/README.md#newcode148 README.md:148: An Android library for the settings UI. This one I'd stretch a bit :D Maybe: "An Android library that provides a configuration interface for Adblock Plus." https://codereview.adblockplus.org/29348393/diff/29374661/README.md#newcode214 README.md:214: An Android library for WebView UI with Adblock Plus integrated. Maybe: "An Android library that provides a WebView component with Adblock Plus integrated." Or something in that vein? Not sure if "component" is the right word here. Maybe "WebView subclass" or something? https://codereview.adblockplus.org/29348393/diff/29374661/README.md#newcode270 README.md:270: An Android application with AdblockWebView usage example. "An Android application that demonstrates how to use AdblockWebView"?
updated patch set https://codereview.adblockplus.org/29348393/diff/29374661/README.md File README.md (right): https://codereview.adblockplus.org/29348393/diff/29374661/README.md#newcode1 README.md:1: Adblock Plus for Android On 2017/02/09 14:31:44, Felix Dahlke wrote: > That's a bit confusing since we have a product called "Adblock Plus for > Android", hm? I think "Adblock Plus Library for Android" is fine, since it's an > Android library project. If you don't like that, how about "Libadblockplus for > Android"? Acknowledged. https://codereview.adblockplus.org/29348393/diff/29374661/README.md#newcode15 README.md:15: An Android library that provides an opportunity to use Adblock Plus core functionality On 2017/02/09 14:31:44, Felix Dahlke wrote: > Seems we could shorten this a bit: > > "An Android library that provides the core functionality of Adblock Plus. You > can find it in the 'libadblockplus-android' directory." Acknowledged. https://codereview.adblockplus.org/29348393/diff/29374661/README.md#newcode88 README.md:88: Android tests for the Library. On 2017/02/09 14:31:44, Felix Dahlke wrote: > Same here: > > "Android tests for the library. You can find them in the > 'libadblockplus-android-tests' directory." Acknowledged. https://codereview.adblockplus.org/29348393/diff/29374661/README.md#newcode148 README.md:148: An Android library for the settings UI. On 2017/02/09 14:31:44, Felix Dahlke wrote: > This one I'd stretch a bit :D Maybe: > > "An Android library that provides a configuration interface for Adblock Plus." Acknowledged. https://codereview.adblockplus.org/29348393/diff/29374661/README.md#newcode214 README.md:214: An Android library for WebView UI with Adblock Plus integrated. On 2017/02/09 14:31:44, Felix Dahlke wrote: > Maybe: > > "An Android library that provides a WebView component with Adblock Plus > integrated." > > Or something in that vein? Not sure if "component" is the right word here. Maybe > "WebView subclass" or something? Acknowledged. https://codereview.adblockplus.org/29348393/diff/29374661/README.md#newcode270 README.md:270: An Android application with AdblockWebView usage example. On 2017/02/09 14:31:44, Felix Dahlke wrote: > "An Android application that demonstrates how to use AdblockWebView"? Acknowledged.
LGTM with the final nit addressed :) https://codereview.adblockplus.org/29348393/diff/29374910/README.md File README.md (right): https://codereview.adblockplus.org/29348393/diff/29374910/README.md#newcode2 README.md:2: ======================== Nit: Please prolong this to match the title above. 
|
