-
Notifications
You must be signed in to change notification settings - Fork 2
WaysToHelpInstructions
- summary Detailed instructions for contributing to AdBlock
- labels Restrict-AddWikiComment-Contributor
These instructions are linked to from tasks on the [WaysToHelp] page.
[WaysToHelp#Verifying_issues] | Relevant Issues
Bug report Issues are labeled `Type-Defect`, and feature request Issues are labeled `Type-Enhancement.`
When making sure an Issue is missing no information:
* If a `Type-Enhancement` doesn't make clear to you exactly what the reporter wants or why they want it, *add a comment* asking those questions ([https://code.google.com/p/adblockforchrome/issues/detail?id=7075 example]). * If a `Type-Defect` is missing any information required by the [http://code.google.com/p/adblockforchrome/issues/entry?template=Defect bug report template], *add a comment* asking for that information. * Example: in [http://code.google.com/p/adblockforchrome/issues/detail?id=6646 this Issue], John asked for the URL where the bug appeared. (He could have asked for more, but the URL was a good start.) * You may be able to think of a question that the template doesn't ask, but that would still help clarify the Issue. *Add a comment* asking your question. * Example: in the Issue above, John could have asked "Does the problem still happen if you disable !AdBlock (in the Chrome menu -> Tools -> Extensions) and restart Chrome?"
- If you're not yet a [#Joining_the_project], stop here.* Project members can do more:
2. You can *add clarifying labels*. If you learn that the issue only applies to a single browser or OS, add a `Browser-` or `OpSys-` label.
3. If the Summary is vague, *improve it* and put the old Summary in a comment. For example, you could update an Issue called "Can't get videos right" by adding a comment saying `[old]` and changing the Summary to `Youtube videos embedded in Facebook posts don't play properly in Safari`.
4. If a `Type-Enhancement` Issue seems clear enough to proceed, *set `Status:Accepted`*, making it a candidate to be [WaysToHelp#Writing_patches].
5. If a `Type-Defect` Issue seems clear enough to proceed, AND has been reproduced, *set `Status:Accepted`*, making it a candidate to be [WaysToHelp#Debugging_issues] and [WaysToHelp#Writing_patches].
6. If the Issue is invalid in some way, you could [WaysToHelp#Triaging_issues].
[WaysToHelp#Verifying_issues]
[WaysToHelp#Verifying_issues] | Relevant Issues
When a user finds a bug, they file a new Issue. The standard bug report template asks the user to give a "repro", which is a recipe for letting someone else reproduce the buggy behavior. A repro contains:
# The exact list of steps to take # What *should* happen after taking those steps (if there were no bug) # What *actually* happens after taking those steps (because there is a bug)
For example, this bug report has good repro instructions. They let the reader know exactly how to make the bug appear. Having a good repro lets you confirm that the bug exists (the user's computer or brain isn't defective) and makes it easier to debug.
1. If the bug report says that a *non-ad is blocked*, that's a special case: no repro is needed. Follow [#Bug_reports:_a_special_case] instead.
2. If the repro is not clear enough to follow, *add a comment* asking for a better repro.
* Note that if a bug is obvious enough, you may not need a formal repro to follow it, for example "The text on mywebsite.com/foo.html turns red when !AdBlock is installed and enabled".
3. If the repro is clear, *follow it and add a comment* with your findings. Include your *OS, browser, and !AdBlock versions*.
* Example of success: "Confirmed with !AdBlock 2.5.40 in Chrome 22 on Windows 7." * Example of failure: "Could not reproduce in Chrome 22/Win7/!AdBlock 2.5.40. The text is black whether !AdBlock is enabled or not."
- If you're not yet a [#Joining_the_project], stop here.* Project members can do more:
5. If the report is invalid in some way, [WaysToHelp#Triaging_issues].
[WaysToHelp#Verifying_issues]
There's a special type of bug report that we see a lot, to the point where you can skip straight to fixing the problem.
If the bug report is that a *non-ad is being blocked* (example, example), then just *add a comment* like this one:
Sorry for the trouble. This sounds like it is caused by a broken filter in one of your filter lists, which are not maintained by AdBlock. Please follow the steps at getadblock.com/faq/hunting to find the broken filter and report it to the correct filter list. When they fix the problem you'll automatically be updated with the fix. Let us know how it goes!Or if you're feeling generous, you could do it yourself, and add a comment like
I followed the steps at getadblock.com/faq/hunting to find that this is caused by the filter ##div.banner which is in the EasyList filter list. I have reported it to EasyList, and when they fix the problem you'll automatically be updated with the fix.
- If you're not yet a [#Joining_the_project], stop here.* Project members, set the `MoreInfoNeeded` label if you asked the user to find the broken filter, or `Status:ReportToList` if you found and reported it yourself.
[WaysToHelp#Investigating_ad_reports] | Relevant Issues
First, try out !AdBlock's built-in ad reporting wizard to see what it does. It's just a bunch of questions. You can run it on any page, including this one, and make up pretend answers, then stop before actually filing an ad report. Do this:
For Safari and Opera 12.15
# Click the !AdBlock toolbar button -> Options # Check the "advanced mode" checkbox # Close the Options page # Click the !AdBlock toolbar button -> Report an ad.
For Chrome and Opera 15
# Click the !AdBlock toolbar button -> Report an ad.
Depending on whether you said you saw the ad in Firefox using Adblock Plus, the wizard either gave you a link to report the ad to a filter list maintainer, or a link to report the ad to us. Doing the latter files a `Type-AdReport` Issue in !AdBlock's Issue Tracker. (Here's an example which the user forgot to fill in with details). We then investigate the report to see how we should proceed.
Follow the steps below when investigating an ad report.
If the user is reporting ads on almost every website, then they ignored the wizard question about "Ads everywhere". *Add a comment* pointing them to getadblock.com/bugs/textenhance, and stop there.
If the report is missing information that you need, *add a comment* requesting more information (example) and stop there.
OK, the report seems complete. Make your copy of !AdBlock have the *same filter lists* as listed in the report. You can do this on the !AdBlock Options page. Then reload the page with the ad.
If you don't see the ad, *add a comment* like "I do not see the ad using !AdBlock 2.5.47 in Chrome 22 on Windows7, with the same filter lists as you. If you Update Filters under !AdBlock Options -> Filter Lists and restart your browser, do you still see the ad?" and stop there.
OK, you see the ad. Now verify that you don't see it in Firefox with Adblock Plus installed, setting ABP's filter lists to match the report's as closely as possible. (You shouldn't use the !AdBlock Custom filter list in ABP.)
If ABP also shows the ad, then this ad report should have never been filed. The user probably lied to the ad reporting wizard and said they had checked in Firefox. *Add a comment* like
This is due to a missing filter in [the report's main filter list for the webpage's language], a filter list which AdBlock does not control. You can report this to them using the contact info on ListAuthorContacts.
But if ABP does not show this ad, then this is a bug in !AdBlock (example). *Add a comment* like
I can see the ad (a text ad at the bottom of the page) in AdBlock 2.5.40 in Chrome 22 on Win7. I do not see it in ABP in Firefox, so I think this is a bug.
- If you're not yet a [#Joining_the_project], stop here.* Project members can do more:
2. If you told the user to report the problem to a filter list, *set `Status:ReportToList`*, which closes the Issue.
3. If the ad appears to be due to a bug in !AdBlock, *set `Status:Accepted`* and change from `Type-AdReport` to *`Type-Defect`*. Along with the comment that you posted, *change the Summary* from "Ad report: google.com" to a better summary of the defect, e.g. "Text ads appearing at bottom of google.com search results pages (blocked in ABP)".
[WaysToHelp#Investigating_ad_reports]
[WaysToHelp#Debugging_issues] | Relevant Issues
!AdBlock is a normal browser extension. You can use Chrome's excellent Developer Tools to debug it. The Console tab lets you review logs and errors; the Elements tab lets you view the DOM layout; and the Sources tab lets you add breakpoints to !AdBlock's code and step through it one step at a time.
about:extensions has a link to !AdBlock's `_generated_background_page.html`, where the bulk of !AdBlock's work happens; the rest happens in content scripts on each webpage. You can open the Developer Tools on any webpage to view !AdBlock's content scripts (in the Sources tab, click the icon at the top left, choose Content Scripts, then click the triangle next to the first item.)
`manifest.json` is the file that describes !AdBlock's structure to Chrome. That's a good place to start to decide which files to look at. If you don't know where to start, but would like to try to find the cause of a defect, leave a comment asking for guidance on the Issue, and someone more experienced can try to point you toward likely places to investigate.
Often it is useful to modify the source code; e.g. to force a function to return true, or to comment out large sections of code to see if it makes a bug go away. To do this, you'll want a local copy of the latest !AdBlock release, which you can get by running
replacing `2.5.40` with [Changelog]. You'll need svn installed on Linux, or TortoiseSVN installed on Windows; more info is here.
Once you have your local copy, go to about:extensions, select Developer Mode -> Load Unpacked Extension, and select the folder containing the local copy. Now !AdBlock is running from that local copy (be sure to disable any other running copies.) Now you can make changes to the code in that folder, then click the Reload link on about:extensions to load the changed version.
If you must debug in Safari, it's a little less pleasant than Chrome. You can use the Developer Tools to view content scripts on a page, but you can't inspect !AdBlock's background page without installing !AdBlock from a local folder. Visit the Developer menu -> Extension Builder -> + sign and select a folder containing !AdBlock's source code. The folder name must end with `.safariextension`, e.g. `adblock-version-2.5.40.safariextension`. (I think you will need a Safari Developer Certificate as well in order to install even this test extension.)
Once the local copy is installed, you can scroll down to "Inspect Global Page" in the Extension Builder to open the Developer Tools on the background page, and click Reload in the Extension Builder after making source code changes.
`Info.plist` is the file that describes !AdBlock to Safari, like Chrome's `manifest.json`. Safari loads some different files from Chrome, so that's a good place to start when looking around.
If you find the cause of an Issue, *add a comment* being as specific as possible about the cause. Remember that whoever comes after you won't have just spent half an hour staring at the code, so she won't have all the context and knowledge in her head that you have right now. For instance, if you found out why Wikipedia keeps turning pink, this comment is not very useful:
I found the problem: We think Wikipedia is loading from msn.com, so it's turning pink.Better, but still not great:
I found the problem: 'domain' is a global variable in the function fix_msn_background, so it's bleeding through to wikipedia.Best:
I found the problem: 'domain' is accidentally defined as a global variable in background.js line 250. If you load Wikipedia in one tab and msn.com in a second tab, then fix_msn_background() is called, 'domain' is set to 'msn.com', and background.js line 400 is triggered when Wikipedia loads its next resource. I expect we could fix it by making 'domain' a local variable, but I haven't tried it.
If you *failed* to find the cause, but learned something, *add a comment* mentioning what you tried and what failed. For example,
I couldn't figure out why Wikipedia keeps turning pink. I tried completely removing the
toolbar button, which didn't make a difference. When it does turn pink, the
<div id="header"> section of the page gets a 'color:pink' style attribute attached, but
I have no idea why. It happens even when I turn off all filter lists, so it can't be a
bad filter. In AdBlock's background.js line 400 it says
if (domain === "msn.com")
page.style.color = "pink";
but Wikipedia doesn't load any resources from msn.com, according to the Developer Tools
Resources tab.
- If you're not yet a [#Joining_the_project], stop here.* Project members can do more:
2. If you find the cause of an issue, *set the `Cause-Known` label*. This lets people who are going to [WaysToHelp#Writing_patches] see that you've saved them the trouble of diagnosing the problem. In fact, they'll see `Cause-Known` Issues listed first, to encourage people to debug Issues that they want to see fixed.
[WaysToHelp#Debugging_issues]
[WaysToHelp#Writing_patches] | Revelant bug reports and feature requests
First, remember that this is a [ProjectOverview#Project_overview], so even if an issue has been reviewed and the consensus is to proceed, Michael might overrule and set `Status:WontFix` (usually on a feature request that he feels is unnecessary). It's frustrating to write lots of code and then find out it's not going to be used! Right now there's no way to tell if Michael has reviewed an issue, so if you're worried about wasting your time, just drop a line to the developer forum and ask him if he would accept a patch addressing the Issue.
Second, be sure to read and *conform to the [#Coding_style]*, or you'll just have to fix your code after it gets reviewed.
If the design of the solution isn't obvious, you could start a thread in the developer forum and point to it in an Issue comment, or just debate the design directly in the Issue comments, until it's clear how to approach the problem.
Here's the process for submitting a patch to address an issue.
# Using svn on Linux or TortoiseSVN on Windows, [http://code.google.com/p/adblockforchrome/source/checkout check out the read-only copy of trunk]. # Make your changes, testing them in the browser (see instructions in [#Detailed_instructions_for:_Debugging_issues "Debugging issues"]). # For huge bonus points, also add unit tests to `tools/tests/test.html`. # Generate a patch ([http://ariejan.net/2007/07/03/how-to-create-and-apply-a-patch-with-subversion svn instructions], [http://tortoisesvn.net/docs/release/TortoiseSVN_en/tsvn-dug-patch.html TortoiseSVN instructions]) # Add a comment to the Issue thoroughly explaining what was changed and what was fixed and how to test it. Add the patchfile as an attachment. # Star the Issue so you are emailed updates.
Someone will review your patch. If there are problems, the Issue will be set to `Status:Started` by a reviewer. In that case, look for feedback in comments on the Issue, argue with the reviewer(s), make changes, and attach a new patch when you're done.
- If you're not yet a [#Joining_the_project], stop here.* Project members can do more:
2. You should set `Status:NeedsReview` when your patch is ready. If there's someone in particular from whom you'd appreciate code review, cc them with your request. After it's reviewed, it may be set to `Status:Started`, and you can set it to `Status:NeedsReview` again after arguing with the reviewer(s) and making changes.
After a few of your patches have been accepted, you should ask for Commit permission in the developer forum, so that you can commit code directly to the source tree instead of attaching patches. This makes writing, reviewing, and collaborating on code easier.
Here's the process for a *Committer* to write code to address an issue:
# Set yourself as `Owner` as described above. # Using svn on Linux or TortoiseSVN on Windows, copy `https://adblockforchrome.googlecode.com/svn/trunk` to `https://adblockforchrome.googlecode.com/svn/branches/issue-5740` (of course with the correct Issue number.) _The first time you do this, you may have to [http://code.google.com/p/adblockforchrome/source/checkout authenticate]._ Then check out a local copy of that folder. # Make your changes, testing them in the browser (see instructions in [#Detailed_instructions_for:_Debugging_issues "Debugging issues"]). # For huge bonus points, also add unit tests to `tools/tests/test.html`. # Check them in to `branches/issue-5740`. You can do this as one giant checkin, or as many small checkins; no one will pay attention until you ask for code review. *Important:* The commit message should start with the line `Update issue 5740`, which will append your commit message as an Issue comment. Be very explicit in your commit messages about what was changed, what was fixed, and how to test it. # Set `Status:NeedsReview`/`cc` as described above, and address `Status:Started` if needed. In addition to feedback in comments, you can get it on specific revisions ([http://code.google.com/p/adblockforchrome/source/detail?r=4590 example]).
Sometimes !AdBlock will need a custom filter in order to fix a problem. Users are subscribed to the !AdBlock Custom filter list, which lives in filters/adblock_custom.txt. If you add a filter there (syntax), it will be shipped the next time Michael ships the filter list; you can cc adblockforchrome to ask him to do so.
[WaysToHelp#Writing_patches]
Code you submit to !AdBlock should conform to the following style rules, unless you have a good reason to break the rules :)
Comment every non-one-liner function that you write with a description of what it does, a list of inputs, and what it returns. See background.js for some examples. When in doubt about whether a chunk of code makes sense, lead it with a comment explaining what's going on. When you implement strange-looking code to handle Issue `1234`, it suffices to say `// Issue 1234` so readers know where to learn more. In general, *err on the side of too many comments*, since dozens of strangers may read your code this year without understanding as much as you do about the context.
Variables should be clearly named instead of short. `EnumsAreNamed.LIKETHIS`; `ClassesAreNamedLikeThis`; `functionsNamedLikeThis`; `_privateFuntionsLikeThis`; `variables_like_this`; `_private_variables_like_this` ("private" meaning "not meant for use outside this object.") When in doubt, follow the capitalization of the code around you.
There are lots of ways in !JavaScript to define a class, its constructor, and its prototype. Follow this format:
Equality should be `===`, not `==`. In regular expressions, put a `\` before every non-special character except `[a-zA-Z0-9_-]` so that it's obvious that all other characters are special characters. E.g. `/^\@\@\|\|ads-r-us\.com(\/ad_content)+$/`
We didn't always have a style guide, so unfortunately the code isn't 100% adherent to the rules above. Refactoring is great -- whenever you touch some code, feel free to clean it up.
Anyone can work anywhere in the code; no one officially owns any area. We use the Owner field to mean "I'm working on this, so if you do too, we will be duplicating effort." Note that anyone is still *allowed* to work on an Issue if they want to, regardless of who is Owner.
Owned issues are listed last in some of the lists of issues, since other project members can probably ignore them. So you should *de-own Issues you aren't actively working on* to get them back in the right place.
When to consider Owning an Issue:
* You're about to start a *complex debugging session*. You set yourself Owner so others don't waste their effort, then de-own it afterwards. * You're about to *write code*. You set yourself Owner so others don't waste their effort, and to be notified of any negative reviews after you set `Status:NeedsReview`. * You want to work on an Issue whose Owner *hasn't updated it for weeks*. You shouldn't de-own an Issue for someone, but you could leave a comment saying "I'm about to work on this. Could you set me as Owner? And have you made any progress that would help me out?"
When to de-own an Issue:
* You're Owner of an Issue that *you decide not to work on* any time in the next week or so. Owning discourages others from working on the Issue; de-own it and leave a comment with any progress you made. (You could cc yourself or star the Issue to not lose track of it.)
[WaysToHelp#Triaging_issues] | Relevant Issues
At a high level, when you triage an Issue you *close it* as `Status:Duplicate/Invalid/WontFix`, or you get it to *`Status:Accepted`*, or you label it *`MoreInfoNeeded`*. You also give it *accurate labels*. It hasn't been triaged until it's out of the triage list.
Let's look at each of those in detail.
You might *close the Issue*:
* Set `Status:Duplicate` if there is *another report of the same issue* ([http://code.google.com/p/adblockforchrome/issues/detail?id=7067 example]). This requires being familiar with all the issues that have arrived lately, or at least searching the older reports. * Set `Status:WaitingOnChrome` (or `Safari` or `Opera`) if the report is due to a known limitation in the browser, e.g. an open bug ([http://code.google.com/p/adblockforchrome/issues/detail?id=7194 example]) or lack of support ([http://code.google.com/p/adblockforchrome/issues/detail?id=7193 example]). * Set `Status:Invalid` if a bug report is clearly *not !AdBlock's fault* (in which case, tell the user what to do instead: [http://code.google.com/p/adblockforchrome/issues/detail?id=5295 example]). This requires understanding the limits of what !AdBlock can and can not do. * Set `Status:Invalid` if the report is in fact *not an !AdBlock issue*, but is e.g. a question (in which case, point the user to the relevant [FrequentlyAskedQuestions FAQ] entry.) * Set `Status:WontFix` if you're positive that this is an issue on which !AdBlock *will not take action* ([http://code.google.com/p/adblockforchrome/issues/detail?id=6766 example]). This requires being with the project for long enough that you've already heard the arguments about why we won't address this issue. * If you aren't sure whether to set `Status:WontFix` or not, then set `MoreInfoNeeded`, cc `adblockforchrome`, and ask Michael's opinion in a comment. (Michael will review all `WontFix`s, so if you're *almost* positive, you could just set `WontFix` and let Michael un-set it if he disagrees; !AdBlock is a [ProjectOverview#Project_overview benevolent dictatorship].) For example, if no one can reproduce a bug that is only affecting one user, we may just close the Issue.
If you don't close the issue, then next you should:
# Correct any incorrect labels (e.g. `Type-Defect`s that should be `Type-Enhancement`s); # [WaysToHelp#Verifying_issues verify the Issue]; and # label it `Priority-Critical`* if it is [#What_is_a_critical_issue? critical].
By this point you will have either set `MoreInfoNeeded` or `Status:Accepted`, moving the Issue out of the triage list. (I can think of an exception: if you can't reproduce an otherwise clear bug report, then all you can do is add a comment and leave the Issue in the triage list for someone else to reproduce or to close.)
[WaysToHelp#Triaging_issues]
An issue is critical and should be labeled `Priority-Critical` (thus sending scary alarm emails to potentially lots of people) if it *is reproducible* (it's not a freak problem on one user's computer) and at least one of the following is true:
* !AdBlock *crashes or stops ad blocking* on all web pages ([http://code.google.com/p/adblockforchrome/issues/detail?id=6360 example], [http://code.google.com/p/adblockforchrome/issues/detail?id=6253 example], [http://code.google.com/p/adblockforchrome/issues/detail?id=6194 example], [http://code.google.com/p/adblockforchrome/issues/detail?id=5384 example]) * !AdBlock *interrupts browsing* for most users ([http://code.google.com/p/adblockforchrome/issues/detail?id=6238 example], [http://code.google.com/p/adblockforchrome/issues/detail?id=6322 example]) * major visible *!AdBlock UI is broken* (e.g. the context menu is gone, or the install or Options page is blank) ([http://code.google.com/p/adblockforchrome/issues/detail?id=6243 example], [http://code.google.com/p/adblockforchrome/issues/detail?id=5389 example]) * it breaks *filter list updates* ([http://code.google.com/p/adblockforchrome/issues/detail?id=1809 example]) * it is a *security bug* allowing XSS or other compromise of user anonymity ([http://code.google.com/p/adblockforchrome/issues/detail?id=5558 example]) * it *affects a giant website*, either by *showing ads* or *breaking a site feature*. "Giant" means a *major* search engine, social network, news outlet, etc -- something used by many millions of users ([http://code.google.com/p/adblockforchrome/issues/detail?id=4362 example], [http://code.google.com/p/adblockforchrome/issues/detail?id=4694 example])
Critical issues can be reported here.
[WaysToHelp#Unlabeling_MoreInfoNeeded_s] | Relevant Issues
`MoreInfoNeeded` Issues are waiting on a user to leave a comment with more information. `MoreInfoNeeded` issues are not shown in most of the lists of Issues, since we can't really take action on them without that information.
So removing the label promptly when the user replies is important. When a user replies to a question in a `MoreInfoNeeded` issue, *remove the label* so it appears in the relevant lists again.
If the user replies to say that the bug no longer appears, *set `Status:Fixed`*.
Also, we give up on waiting after 14 days. If the user has not replied by then, *set `Status:Invalid`* and add the comment
Over 14 days ago we requested some additional information, but we didn't get any. If you still have this problem, feel free to open a new Issue and mention this Issue's number.
You can do this in bulk. Control-click each over-14-days-old issue in the list to open it in a new tab. Check its checkbox in the list after verifying there's no response. When you have several checked, use the Actions... dropdown -> Bulk Edit to set the Status and add the comment to all of them at once.
[WaysToHelp#Unlabeling_MoreInfoNeeded_s]
[WaysToHelp#Integrating_translations] | Relevant Issues
First, you'll need to be a Committer to integrate translations. If you aren't one yet, just ask in the development forum to become a Committer to help with translations. You'll also need !AdBlock's trunk source code. Install TortoiseSVN on Windows, then check out `https://adblockforchrome.googlecode.com/svn/trunk/`, specifying your Google account as username and googlecode.com password as password.
Second, read HowToTranslate to see how translators create `messages.json` files to attach to the Issue for their locale when they make a change.
When you have an updated `messages.json` file, you:
# Replace your copy of `trunk/_locales/(some locale)/messages.json` file with the new one. (For example, the French locale's translation is at [http://code.google.com/p/adblockforchrome/source/browse/trunk/_locales/fr trunk/_locales/fr/messages.json].) # Check for problems in your modified `trunk` # Either *comment on the Issue* explaining the problems you found, and undo your local changes; or *commit your changes* to trunk.
If the `messages.json` file has a problem, you should *add a comment* to the Issue telling the translator how to fix it. Here's how to deal with several potential problems.
1. If there is an active maintainer of a translation, don't accept a *different person's* translation until you check with the active maintainer that it's OK with them. We trust the active maintainer more than a stranger. If the maintainer doesn't respond, the new person can become the active maintainer.
2. If the user didn't use http://chromeadblock.com/l10n/ to create their file, the *syntax* may be incorrect. You can check that by double clicking `ValidateMessages.json.exe`, which is found in the `trunk/tools` folder. That program reads all the `messages.json` files in your `trunk/_locales` directory and displays any errors found. (Don't forget to copy the new `messages.json` file in place before you do this.) Scroll down to the correct `messages.json` file and check for any errors listed, and let the translator know how to fix them, or fix them yourself.
(You can also click the "Remove unused strings" button in `ValidateMessages.json.exe` to delete strings that aren't detected as used in !AdBlock nor listed as special cases in `trunk/tools/I18N_include_exclude.txt`. This will modify many files at once, and in that case you should commit the entire `_locales` directory with a commit message like "Removed unused strings".)
3. If the user accidentally or intentionally added a *bad translation*, we must catch that. To see exactly what the user has changed, use TortoiseSVN's diff command. Copy and paste their translations into Google Translate to make sure they sound about right.
Don't let the translator change the meaning of the English sentence or add their own commentary. For example, don't let them translate "!EasyList" into "!EasyList (not recommended)" or "!EasyList - email me at xyz if you have problems". Don't let them translate a long paragraph full of details with a short summary lacking those details.
4. If anything else *looks "weird"*, don't allow it. (If you're unsure, cc `adblockforchrome` on the Issue and ask Michael's opinion.)
If you find any problems, use TortoiseSVN to revert your changes.
Once there are no problems with the changes you have made to `trunk/_locales/(some locale)/messages.json`, you commit it using TortoiseSVN, adding a commit message that looks like
`Update Issue 5439`
`Thanks! Your changes will appear in the next version.`
The first line of the commit message makes this appear as a comment on the Issue.
After committing all the changes, don't forget to update the list of missing translations on the HowToTranslate wiki page (run `ValidateMessages.json.exe` to generate the list again).
If new strings were added to !AdBlock, leave a comment on the HowToTranslate page so people who starred it will get notified by email.
[WaysToHelp#Integrating_translations]
[WaysToHelp#Reviewing_code] | Relevant Issues
To review code that is in a branch:
# Look through the Issue's comments for each revision checkin. If the author did things correctly, every revision should have resulted in a comment. # Click each revision and then click 'Expand all' to see the code changes in that revision. Or, check out a local copy of trunk and do something like `svn merge --reintegrate https://adblockforchrome.googlecode.com/svn/branches/issue-5740 && svn diff` in order to view the combined effect of all the revisions. # If the change is non-trivial, check out a copy of the branch with something like `svn checkout http://adblockforchrome.googlecode.com/svn/branches/issue-5740 adblock-issue-5740-for-codereview` and test it in the browser (see instructions in [#Detailed_instructions_for:_Debugging_issues "Debugging issues".]) # If the change includes unit tests, open tools/tests/test.html in your browser to run them. # Check that the code adheres to the [#Coding_style style guide]. # Leave comments with suggestions/bugs/questions in the "General comment" field for any specific revision, or double click any line of code in a revision to leave an in-line comment ([http://code.google.com/p/adblockforchrome/source/detail?r=4590 example]). You can also give an overall Positive or Negative review score.
To review a patch submitted by a non-Committer, you'll have to just read the patch and leave a comment in the Issue with your feedback. To test it, you'd have to apply the patch to a local copy of trunk.
If you left review comments but *don't feel confident* enough to decide whether to ship the code (or prevent it from shipping), you're done. The author and other reviewer(s) will take your comments into consideration.
If your review was *positive*, all issues raised by other reviewers have been settled, you've tested the code, and you are confident that it is ready to ship to millions of users, then *set `Status:ReadyForRelease`*. *Don't do this lightly!* Michael will then merge it to trunk and set `Status:Fixed`, and it will be shipped in the next release.
If you think that *the code needs work* before shipping, set `Status:Started`. Be sure that the negative code review(s) contain specific points that needed to be addressed, so that the author knows how to proceed. (The author will probably address the concerns you raised by changing code, but on some of the points you may debate and then decide that no change is necessary. For example, if you suggest a refactoring, and the author feels the change wouldn't add much benefit, and the style guide is not violated, well, there's no strong argument either way... so make the change yourself and get it reviewed, or let the matter drop.)
[WaysToHelp#Reviewing_code]
To contribute to !AdBlock, all you need is a Google account. With that, you can comment on Issues in !AdBlock's Issue Tracker, which lets you help in basic ways.
But you're limited because you can't edit Issues in more powerful ways, which only *project members* can do. Everything you do to help can be done more effectively as a project member, and some important tasks can't be done at all by non-members.
Luckily, it's easy to join the project, and you're encouraged to do so! Once you've worked on, say, more than a dozen issues over more than a week, just ask in the developer forum to join the project. We'll make you an official member of the !AdBlock project after we check that you've been updating Issues correctly.
Once you've become a project member, read the instructions [#Project_members] to learn how to use your new powers :)
[WaysToHelp]
Project members start out as "Contributors", and may become "Committers" if they would like. All Contributors and Committers are listed on !AdBlock's People page.
Contributors can edit an Issue's *status* (whether it's newly filed, duplicate, fixed, etc), *summary* (like "!AdBlock is breaking GMail"), and *labels* (tags applied to the Issue that mean things like "This only happens in Windows", "Waiting for the user to reply", or "This is a serious bug".) You'll see fields to do this when you're adding a comment to an Issue. (If you don't, make sure you're logged in to Google with the correct email address.)
Here are the common statuses and labels that we use, and what they mean. You can set a status or label whenever it's appropriate.
|| *Open Statuses* || *Meaning* || || `New` || Not yet reproduced (for Defects/!AdReports) or understood (for Enhancements). || || `Accepted` || Reproduced or understood, and not closed as `Invalid`/`Duplicate`/`WontFix`. || || `NeedsReview` || Code has been written and is ready for code review. || || `ReadyForRelease` || Someone reviewed the code and is confident that it code is ready to merge into trunk. || || `Started` || Someone reviewed the code and in their opinion it still needs work. ||
|| *Closed Statuses* || *Meaning* || || `Fixed` || Code to address the Issue is in trunk. || || `Invalid` || Insufficient data / not a valid issue || || `Duplicate` || This report duplicates an existing Issue || || `WontFix` || We decided to not take action on this issue || || `WaitingOn[Browser]` || Can't address this without support from `Browser`, e.g. Chrome || || `ReportToList` || This `AdReport` should be reported to a filter list maintainer ||
|| *Labels* || *Meaning* || || `Browser-Chrome` || This Issue *only* applies to Chrome (or `-Safari` or `-Opera`) || || `OpSys-OSX` || This Issue *only* applies to Mac OS X (or `-Windows` or `-Linux`) || || `Type-AdReport` || Ad report || || `Type-Enhancement` || Feature request || || `Type-Defect` || Bug report || || `Cause-Known` || The root cause of this `Type-Defect` has been found || || `Priority-Critical` || This is a [#What_is_a_critical_issue?] Issue (this label sends scary alarm emails) || || `MoreInfoNeeded` || We are waiting on a reply from a user ||
_If you haven't yet joined the project as an official Contributor, [#Joining_the_project]!_
Committers have the same powers as Contributors, but can also commit code directly to !AdBlock's source tree, which makes code review easier.
_After you have had a few patches accepted into !AdBlock, ask in the development forum to become a Committer!_