Issue 5648419829841920: Issue 131 - [Document projects consistently] Add a README.md to the adblockpluschrome repository (Closed)
|
Created:
Feb. 16, 2015, 10:48 a.m. by Wladimir Palant Modified:
Feb. 17, 2015, 1:52 p.m. Visibility:
Public. |
DescriptionIssue 131 - [Document projects consistently] Add a README.md to the adblockpluschrome repository
Patch Set 1 #
Total comments: 13
Patch Set 2 : Addressed comments #MessagesTotal messages: 7
http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... File README.md (right): http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... README.md:61: The unit test will run automatically once the page loads. Should be "unit tests" here of course.
http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... File README.md (right): http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... README.md:3: I think usually a brief introduction at the start of README files is useful to quickly explain what the repository does and why. http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... README.md:23: This will create a build with a name in the form Maybe mention something about how ensure_dependencies is called and pulls in a few other repositories? (Might be useful to list them too.) http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... README.md:32: ### Development environment Maybe the development environment section should be above the building section. It's a more common thing for new people to want to do. http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... README.md:41: This will create a _devenv_ directoy in the repository. In Chrome and Opera you I would use backticks ` instead of underscores _ for strings like "devenv". http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... README.md:61: The unit test will run automatically once the page loads. Maybe add details of the code license(s) to the end of the readme here?
http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... File README.md (right): http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... README.md:10: - [The Jinja2 module](http://jinja.pocoo.org/docs) PIL (Python Imaging Library) is required as well. http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... README.md:28: Apple Developer Center first. _adblockplussafari.pem_ should contain your adblockplussafari.pem must also contain the key for your certificate. http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... README.md:57: extension's Options page, open JavaScript Console and type in: Nit: open the JavaScript Console ^
I addressed most comments, replied to some. http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... File README.md (right): http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... README.md:23: This will create a build with a name in the form On 2015/02/17 09:38:05, kzar wrote: > Maybe mention something about how ensure_dependencies is called and pulls in a > few other repositories? (Might be useful to list them too.) I don't really want to document internals of buildtools here. I've added a reference to the "dependencies" file however so whoever is interested can figure out how this works. Listing dependent repository: definitely not, that would duplicate state and have the tendency to go out of sync. http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... README.md:32: ### Development environment On 2015/02/17 09:38:05, kzar wrote: > Maybe the development environment section should be above the building section. > It's a more common thing for new people to want to do. Most people seem to start by generating an installable build with a minor modification. The development environment only becomes useful once you make many changes. http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... README.md:41: This will create a _devenv_ directoy in the repository. In Chrome and Opera you On 2015/02/17 09:38:05, kzar wrote: > I would use backticks ` instead of underscores _ for strings like "devenv". Blame fhd - he started doing that (probably because he dislikes code formatting), and now we have to keep doing it for consistency ;) http://codereview.adblockplus.org/5648419829841920/diff/5629499534213120/READ... README.md:61: The unit test will run automatically once the page loads. On 2015/02/17 09:38:05, kzar wrote: > Maybe add details of the code license(s) to the end of the readme here? This doesn't belong in the README file, it should be a separate LICENSE or COPYING file.
LGTM
LGTM 
|
