from: https://code.google.com/archive/p/adblockforchrome/wikis/WaysToHelpInstructions.wiki
adblockforchrome - WaysToHelpInstructions.wiki
Ways to help: Detailed instructions
These instructions are linked to from tasks on the Ways To Help page.
<hr>
<hr>
Detailed instructions for: Filling in missing information
Back to overview | 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 (example). * If a Type-Defect
is missing any information required by the bug report template, add a comment asking for that information. * Example: in 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 project member, stop here. Project members can do more:
-
If you asked a question, you should set the
MoreInfoNeeded
label. This label lets others know that we're waiting to hear back from the reporter. It also removes the Issue from most of the "Ways to help" lists, so consider also starring the Issue. That way you'll be emailed as soon as a response comes back and you could remove the label. -
You can add clarifying labels. If you learn that the issue only applies to a single browser or OS, add a
Browser-
orOpSys-
label. -
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 Summary: "Can't get videos right"]
and changing the Summary toYoutube videos embedded in Facebook posts don't play properly in Safari
. -
If a
Type-Enhancement
Issue seems clear enough to proceed, setStatus:Accepted
, making it a candidate to be implemented. -
If a
Type-Defect
Issue seems clear enough to proceed, AND has been reproduced, setStatus:Accepted
, making it a candidate to be debugged and fixed. -
If the Issue is invalid in some way, you could triage it.
Back to overview
Detailed instructions for: Reproducing bugs
Back to overview | http://code.google.com/p/adblockforchrome/issues/list?can=2&q=type=defect%20Status:New%20-MoreInfoNeeded&sort=modified&colspec=ID%20Opened%20Modified%20Type%20Summary'>Relevant Issues
What is a repro?When a user finds a bug, they file a new Issue. The http://code.google.com/p/adblockforchrome/issues/entry?template=Defect'>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, https://code.google.com/p/adblockforchrome/issues/detail?id=7053'>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.
- If the bug report says that a non-ad is blocked, that's a special case: no repro is needed. Follow these instructions instead.
- 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".
- 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 project member, stop here. Project members can do more:
4. If you were able to reproduce the bug, set Status:Accepted
, making it a candidate to be debugged and fixed.
5. If the report is invalid in some way, triage it.
Back to overview
Bug reports: a special case
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 (http://code.google.com/p/adblockforchrome/issues/detail?id=6422'>example, http://code.google.com/p/adblockforchrome/issues/detail?id=6643'>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.Bonus instructions If you're not yet a project member, 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.
Back
Detailed instructions for: Investigating ad reports
Back to overview | Relevant IssuesThe wizardFirst, 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.
- Click the AdBlock toolbar button -> Report an ad.
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.
How to investigateFollow the steps below when investigating an ad report.
Step 1: Handle invalid reportsIf 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.
Step 2: Try to find the adOK, 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.
Step 3: Is there a missing filter?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.Bonus instructions If you're not yet a project member, stop here. Project members can do more:
1. If you added a comment and were not able to see the ad, set the
MoreInfoNeeded
label, which will take the ad report out of the "investigate" list until the user replies.
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)".
Back to overview
Detailed instructions for: Debugging issues
Back to overview | Relevant IssuesSeeing AdBlock in actionAdBlock 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.
Editing the sourceOften 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
svn checkout http://adblockforchrome.googlecode.com/svn/tags/2.5.40 adblockforchrome-read-only
replacing
2.5.40
with the current release number. 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.
Debugging in SafariIf 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.
When you're finished debuggingIf 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 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.Bonus instructions
If you're not yet a project member, stop here. Project members can do more:
1. If you expect debugging to take a while, you should set yourself as Owner
(see the definition), so that others don't duplicate your work.
2. If you find the cause of an issue, set the Cause-Known
label. This lets people who are going to write code 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.
Back to overview
Detailed instructions for: Writing patches
Back to overview | Revelant
http://code.google.com/p/adblockforchrome/issues/list?can=2&q=Type=Defect%20Status:Accepted%20-MoreInfoNeeded&sort=-Owner+Cause+Modified&colspec=Cause%20ID%20Opened%20Modified%20Status%20Summary%20Owner'>bug reports and http://code.google.com/p/adblockforchrome/issues/list?can=2&q=Type=Enhancement%20Status:Accepted%20-MoreInfoNeeded&sort=-Owner+Modified&colspec=ID%20Opened%20Modified%20Status%20Summary%20Owner'>feature requests
First, remember that this is a benevolent dictatorship, 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 http://groups.google.com/group/adblockforchrome-dev'>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 guide, 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 http://groups.google.com/group/adblockforchrome-dev'>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, check out the read-only copy of trunk.
- Make your changes, testing them in the browser (see instructions in "Debugging issues").
- For huge bonus points, also add unit tests to
tools/tests/test.html
.
- Generate a patch (svn instructions, 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 project member, stop here. Project members can do more:
1. You should set yourself as Owner
(see the definition) before you start working, so that others don't duplicate your work; and
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 http://groups.google.com/group/adblockforchrome-dev'>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
tohttps://adblockforchrome.googlecode.com/svn/branches/issue-5740
(of course with the correct Issue number.) The first time you do this, you may have to authenticate. Then check out a local copy of that folder.
- Make your changes, testing them in the browser (see instructions in "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 lineUpdate 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 addressStatus:Started
if needed. In addition to feedback in comments, you can get it on specific revisions (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 (http://adblockplus.org/en/filters'>syntax), it will be shipped the next time Michael ships the filter list; you can cc adblockforchrome to ask him to do so.
Back to overview
Coding style
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:
function ClassName(x, y) {
this._privateVar1 = x;
this.publicVar2 = y;
}
ClassName.staticMethod = function(x) {
};
ClassName.prototype = {
method1: function(x) {
},
method2: function(x) {
}
};
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.
The Owner
field
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.)
Detailed instructions for: Triaging issues
Back to overview | http://code.google.com/p/adblockforchrome/issues/list?can=2&q=type=defect,enhancement%20Status:New%20-MoreInfoNeeded&sort=modified&colspec=ID%20Opened%20Modified%20Type%20Summary'>Relevant Issues
What exactly is triage?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 http://code.google.com/p/adblockforchrome/issues/list?can=2&q=type=defect,enhancement%20Status:New%20-MoreInfoNeeded&sort=modified&colspec=ID%20Opened%20Modified%20Type%20Summary'>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 (example). This requires being familiar with all the issues that have arrived lately, or at least searching the older reports.
- Set
Status:WaitingOnChrome
(orSafari
orOpera
) if the report is due to a known limitation in the browser, e.g. an open bug (example) or lack of support (example).
- Set
Status:Invalid
if a bug report is clearly not AdBlock's fault (in which case, tell the user what to do instead: 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 FAQ entry.)
- Set
Status:WontFix
if you're positive that this is an issue on which AdBlock will not take action (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 setMoreInfoNeeded
, ccadblockforchrome
, and ask Michael's opinion in a comment. (Michael will review allWontFix
s, so if you're almost positive, you could just setWontFix
and let Michael un-set it if he disagrees; AdBlock is a 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 beType-Enhancement
s);
- verify the Issue; and
- label it
Priority-Critical
if it is critical.
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.)
Back to overview
What is a critical issue?
An issue is critical and should be labeledPriority-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 (example, example, example, example)
- AdBlock interrupts browsing for most users (example, example)
- major visible AdBlock UI is broken (e.g. the context menu is gone, or the install or Options page is blank) (example, example)
- it breaks filter list updates (example)
- it is a security bug allowing XSS or other compromise of user anonymity (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 (example, example)
Detailed instructions for: Unlabeling MoreInfoNeeded
s
Back to overview | 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.
Back to overview
Detailed instructions for: Integrating translations
Back to overview | Relevant IssuesFirst, 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 outhttps://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 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/'>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 http://tortoisesvn.net/docs/nightly/TortoiseSVN_en/tsvn-dug-wcstatus.html#tsvn-dug-diffing'>diff command. Copy and paste their translations into http://translate.google.com/#auto/en/'>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 http://tortoisesvn.net/docs/nightly/TortoiseSVN_en/tsvn-dug-revert.html'>revert your changes.
Once there are no problems with the changes you have made to trunk/_locales/(some locale)/messages.json
, you http://tortoisesvn.net/docs/nightly/TortoiseSVN_en/tsvn-dug-commit.html'>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.
Back to overview
Detailed instructions for: Reviewing code
Back to overview | http://code.google.com/p/adblockforchrome/issues/list?can=2&q=status:needsreview&sort=modified&colspec=ID%20Opened%20Modified%20Owner%20Summary'>Relevant Issues
How to perform the code reviewTo 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 "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 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 (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.)
Back to overview
Joining the project
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 http://groups.google.com/group/adblockforchrome-dev'>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 below to learn how to use your new powers :)
Home
Project members
Project members start out as "Contributors", and may become "Committers" if they would like. All Contributors and Committers are listed on AdBlock's http://code.google.com/p/adblockforchrome/people/list'>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.
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 critical 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, go for it!
CommittersCommitters 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 http://groups.google.com/group/adblockforchrome-dev'>development forum to become a Committer!