Issue 29442555: Issue 4934 - Add 'Requirements for Adblock Plus recommended filter lists' page to adblockplus.org (Closed)
|
Created:
May 19, 2017, 5:09 a.m. by ire Modified:
Sept. 7, 2017, 10:35 a.m. CC:
wspee Base URL:
https://hg.adblockplus.org/web.adblockplus.org Visibility:
Public. |
DescriptionIssue 4934 - Add 'Requirements for Adblock Plus recommended filter lists' page to adblockplus.org
Patch Set 1 #
Total comments: 8
Patch Set 2 : Added missing <fix>, Remove space around string brackets, Replace angled quotes #
Total comments: 4
Patch Set 3 : Addressed NITs #
MessagesTotal messages: 9
Thanks Ire! :) There are a few minor issues. Note: When I find a NIT (e.g. missing <fix>, unnecessary UTF-8) I leave a comment on one line (not everywhere it occurs). So please check the rest of the changeset for more occurrences. https://codereview.adblockplus.org/29442555/diff/29442556/pages/contribute.html File pages/contribute.html (right): https://codereview.adblockplus.org/29442555/diff/29442556/pages/contribute.ht... pages/contribute.html:213: {{ filter-lists-requirements-paragraph[Filter Lists Requirements Paragraph] Adblock Plus is only as good as its filters. Share your filters, suggest new ones or help communities of volunteer contributors to draft and update filter lists. These filter lists need to be constantly maintained to block menaces like new tracking systems, malware threats and intrusive ads. Click <a href="https://adblockplus.org/filter-lists-requirements">here</a> for a list of requirements. }} NIT: Please remove "https://adblockplus.org/" from this url. cms HTMLConverter automatically resolves and localizes anchors by page name and language. https://codereview.adblockplus.org/29442555/diff/29442556/pages/filter-lists-... File pages/filter-lists-requirements.html (right): https://codereview.adblockplus.org/29442555/diff/29442556/pages/filter-lists-... pages/filter-lists-requirements.html:4: <p>{{ filter-lists-requirements-intro[Filter Lists Requirements Intro] The list: }}</p> NIT: Please remove spaces before string IDs ( PS: I had no idea that we could put a space there :D - We should be consistent with other pages anyhow...) https://codereview.adblockplus.org/29442555/diff/29442556/pages/filter-lists-... pages/filter-lists-requirements.html:15: {{ filter-lists-requirements-item2-example[Filter Lists Requirements List Item] Example: we will not support a second German filter list, or a list that has filters already contained in EasyList (i.e. do not create another “EasyList Germany” if it does not include additional or better filters) }} NIT: Missing <fix> around EasyList https://codereview.adblockplus.org/29442555/diff/29442556/pages/filter-lists-... pages/filter-lists-requirements.html:24: {{ filter-lists-requirements-item3-example[Filter Lists Requirements List Item] Example: should have no redundancy, no extremely generic filters, no <a href="https://adblockplus.org/forum/viewtopic.php?t=6118">“slow” filters</a> }} NIT: Please replace angled commas and quotes
I realise you probably didn't see this. I had pushed the new patch a month ago but didn't send a message :/ https://codereview.adblockplus.org/29442555/diff/29442556/pages/contribute.html File pages/contribute.html (right): https://codereview.adblockplus.org/29442555/diff/29442556/pages/contribute.ht... pages/contribute.html:213: {{ filter-lists-requirements-paragraph[Filter Lists Requirements Paragraph] Adblock Plus is only as good as its filters. Share your filters, suggest new ones or help communities of volunteer contributors to draft and update filter lists. These filter lists need to be constantly maintained to block menaces like new tracking systems, malware threats and intrusive ads. Click <a href="https://adblockplus.org/filter-lists-requirements">here</a> for a list of requirements. }} On 2017/05/30 12:55:02, juliandoucette wrote: > NIT: Please remove "https://adblockplus.org/" from this url. cms HTMLConverter > automatically resolves and localizes anchors by page name and language. Done. https://codereview.adblockplus.org/29442555/diff/29442556/pages/filter-lists-... File pages/filter-lists-requirements.html (right): https://codereview.adblockplus.org/29442555/diff/29442556/pages/filter-lists-... pages/filter-lists-requirements.html:4: <p>{{ filter-lists-requirements-intro[Filter Lists Requirements Intro] The list: }}</p> On 2017/05/30 12:55:02, juliandoucette wrote: > NIT: Please remove spaces before string IDs > > ( PS: I had no idea that we could put a space there :D - We should be consistent > with other pages anyhow...) Done. https://codereview.adblockplus.org/29442555/diff/29442556/pages/filter-lists-... pages/filter-lists-requirements.html:15: {{ filter-lists-requirements-item2-example[Filter Lists Requirements List Item] Example: we will not support a second German filter list, or a list that has filters already contained in EasyList (i.e. do not create another “EasyList Germany” if it does not include additional or better filters) }} On 2017/05/30 12:55:03, juliandoucette wrote: > NIT: Missing <fix> around EasyList Done. https://codereview.adblockplus.org/29442555/diff/29442556/pages/filter-lists-... pages/filter-lists-requirements.html:24: {{ filter-lists-requirements-item3-example[Filter Lists Requirements List Item] Example: should have no redundancy, no extremely generic filters, no <a href="https://adblockplus.org/forum/viewtopic.php?t=6118">“slow” filters</a> }} On 2017/05/30 12:55:03, juliandoucette wrote: > NIT: Please replace angled commas and quotes Done.
LGTM with minor NITs. @Tamara do we want to delay publishing this until we have translations? (I don't remember a rule about this - if there is or should be one, can you please remind me?) https://codereview.adblockplus.org/29442555/diff/29452271/pages/filter-lists-... File pages/filter-lists-requirements.html (right): https://codereview.adblockplus.org/29442555/diff/29452271/pages/filter-lists-... pages/filter-lists-requirements.html:8: {{filter-lists-requirements-item[Filter Lists Requirements List Item] Should be open source and have the possibility for users to report issues/suggest filters}} NIT: We usually start with "1" instead of nothing. https://codereview.adblockplus.org/29442555/diff/29452271/pages/filter-lists-... pages/filter-lists-requirements.html:12: {{filter-lists-requirements-item2[Filter Lists Requirements List Item] Must be unique (does not duplicate another list or filters in another list)}} NIT: We usually separate numbers by a "-" in string IDs.
On 2017/07/03 22:18:23, juliandoucette wrote: > LGTM with minor NITs. > > @Tamara do we want to delay publishing this until we have translations? > > (I don't remember a rule about this - if there is or should be one, can you > please remind me?) > > https://codereview.adblockplus.org/29442555/diff/29452271/pages/filter-lists-... > File pages/filter-lists-requirements.html (right): > > https://codereview.adblockplus.org/29442555/diff/29452271/pages/filter-lists-... > pages/filter-lists-requirements.html:8: {{filter-lists-requirements-item[Filter > Lists Requirements List Item] Should be open source and have the possibility for > users to report issues/suggest filters}} > NIT: We usually start with "1" instead of nothing. > > https://codereview.adblockplus.org/29442555/diff/29452271/pages/filter-lists-... > pages/filter-lists-requirements.html:12: > {{filter-lists-requirements-item2[Filter Lists Requirements List Item] Must be > unique (does not duplicate another list or filters in another list)}} > NIT: We usually separate numbers by a "-" in string IDs. Please note that I didn't know about any of this and I wasn't CC'ed in the issue either, so not sure how long until I have the translations. Considering it's a new page for the website and everything has to be done "from scratch", it shouldn't be a problem if we have it live at first in English. There isn't really any rule (at least at the moment) about how to do this, unless it involves for example a legal text that cannot wait to be live, be it for a specific language or even just in English. I guess it's more a case by case basis.
Thanks Tamara :) - Please find follow up questions below. CC Winsley (because this is relevant to project management) On 2017/07/04 12:21:12, tamara wrote: > Please note that I didn't know about any of this and I wasn't CC'ed in the issue > either, so not sure how long until I have the translations. I think this implies that we should CC you on every issue that involves a text change on a website that is translated. If so, I'm concerned about spamming you with every minutiae change to such issues. Perhaps we could come up with a more efficient process? > Considering it's a new page for the website and everything has to be done "from > scratch", it shouldn't be a problem if we have it live at first in English. What about the change to the existing page that links to the new page? If we push it, there will be one english paragraph on this page in every language. I don't know if we want that or not. What do you think?
On 2017/07/04 15:19:25, juliandoucette wrote: > Thanks Tamara :) - Please find follow up questions below. > > CC Winsley (because this is relevant to project management) > > On 2017/07/04 12:21:12, tamara wrote: > > Please note that I didn't know about any of this and I wasn't CC'ed in the > issue > > either, so not sure how long until I have the translations. > > I think this implies that we should CC you on every issue that involves a text > change on a website that is translated. If so, I'm concerned about spamming you > with every minutiae change to such issues. Perhaps we could come up with a more > efficient process? > > > Considering it's a new page for the website and everything has to be done > "from > > scratch", it shouldn't be a problem if we have it live at first in English. > > What about the change to the existing page that links to the new page? If we > push it, there will be one english paragraph on this page in every language. I > don't know if we want that or not. What do you think? Regarding your first question: I completely understand, so I would suggest to create an issue apart in the Hub where the need for translations for that specific issue can be brought up. This would help me to keep that lined up in my ongoing projects so that, as soon as the English can be considered final, I can then start requesting the translations. Would that help? The Hub is basically a tool for people to let me know they need translations and for me to have an idea of what's next in the pipeline, so I can organise my time and different projects and at least you wouldn't spam me at all. :) Regarding the second one, I'd suggest to push that specific change affecting "abp.org/contribute#filters" as "update_without_changes". It will be then updated for English while the rest of the languages will still keep and display the current translations until the new ones are ready for implementation which, in my opinion, is better than just displaying English. How does that sound?
On 2017/07/05 09:24:49, tamara wrote: > Regarding the second one, I'd suggest to push that specific change affecting > "abp.org/contribute#filters" as "update_without_changes". It will be then > updated for English while the rest of the languages will still keep and display > the current translations until the new ones are ready for implementation which, > in my opinion, is better than just displaying English. How does that sound? SGTM @ire you can push this (preferably addressing NITs above beforehand).
> @ire you can push this (preferably addressing NITs above beforehand). Okay https://codereview.adblockplus.org/29442555/diff/29452271/pages/filter-lists-... File pages/filter-lists-requirements.html (right): https://codereview.adblockplus.org/29442555/diff/29452271/pages/filter-lists-... pages/filter-lists-requirements.html:8: {{filter-lists-requirements-item[Filter Lists Requirements List Item] Should be open source and have the possibility for users to report issues/suggest filters}} On 2017/07/03 22:18:23, juliandoucette wrote: > NIT: We usually start with "1" instead of nothing. Done. https://codereview.adblockplus.org/29442555/diff/29452271/pages/filter-lists-... pages/filter-lists-requirements.html:12: {{filter-lists-requirements-item2[Filter Lists Requirements List Item] Must be unique (does not duplicate another list or filters in another list)}} On 2017/07/03 22:18:23, juliandoucette wrote: > NIT: We usually separate numbers by a "-" in string IDs. Done. 
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
