Last Comment Bug 471588 - Remove String docs
: Remove String docs
Product: Core
Classification: Components
Component: String (show other bugs)
: Trunk
: All All
: -- minor (vote)
: ---
Assigned To: Tyler Downer [:Tyler]
: Nathan Froyd [:froydnj]
Depends on:
Blocks: 70076 471144
  Show dependency treegraph
Reported: 2008-12-30 14:14 PST by Tyler Downer [:Tyler]
Modified: 2012-03-12 15:12 PDT (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch v1 (90.10 KB, patch)
2008-12-30 14:14 PST, Tyler Downer [:Tyler]
dbaron: review+
Details | Diff | Splinter Review
Patch with comments addressed (90.50 KB, patch)
2011-05-05 17:07 PDT, Tyler Downer [:Tyler]
tdowner: review+
Details | Diff | Splinter Review

Description Tyler Downer [:Tyler] 2008-12-30 14:14:38 PST
Created attachment 354878 [details] [diff] [review]
patch v1

Per bug 471144, there is probably no need for the docs to be in string. Especially, quote
>This document is now deprecated in favor of The new string guide >(
We could just remove, and make any likns, no need to have old docs in the source when not needed.
Comment 1 David Baron :dbaron: ⌚️UTC-10 2011-04-25 11:45:10 PDT
Comment on attachment 354878 [details] [diff] [review]
patch v1

Please leave xpcom/string/README.html; that shows up in, which is useful (we should have more of them).

I still need to look through the doc and see what, if anything, should be migrated to our current docs.
Comment 2 Justin Lebar (not reading bugmail) 2011-04-25 12:58:01 PDT
You mean this?

I have no stake in removing or keeping this, but as far as adding more like it, I'm curious: How is it useful?
Comment 3 David Baron :dbaron: ⌚️UTC-10 2011-05-05 12:07:14 PDT
Comment on attachment 354878 [details] [diff] [review]
patch v1

Yeah; these should go.

At some point I should probably revise a bit, but I don't think that blocks removing these.

Sorry for taking so long to get to this.
Comment 4 David Baron :dbaron: ⌚️UTC-10 2011-05-05 12:09:49 PDT
The README.html is useful because it gives information about what's in a directory, just like comments at top of file give information about what's in a file.  That's particularly useful when the comments are reliably present, as in
Comment 5 Tyler Downer [:Tyler] 2011-05-05 12:19:05 PDT
So are we removing or keeping the README.html?
Comment 6 David Baron :dbaron: ⌚️UTC-10 2011-05-05 13:17:42 PDT
keep xpcom/string/README.html; remove xpcom/string/doc/README.html
Comment 7 Tyler Downer [:Tyler] 2011-05-05 17:07:27 PDT
Created attachment 530467 [details] [diff] [review]
Patch with comments addressed

Ok, put the readme back, pulling down the r+
Comment 8 Mounir Lamouri (:mounir) 2011-05-06 02:08:08 PDT
I tried to push this to cedar but the patch failed to apply. I'm not removing the checkin-needed keyword given that it might apply on mozilla-central. But Tyler, can you have a look and update the patch if needed?
Comment 9 Mounir Lamouri (:mounir) 2011-05-06 02:08:40 PDT
BTW, a patch with a commit message and author name might be appreciated :)
Comment 10 Daniel Holbert [:dholbert] 2011-05-06 12:41:13 PDT
(In reply to comment #8)
> given that it might apply on mozilla-central.

It doesn't -- but it's not due to bitrot.

Tyler's patch has two issues (found from hg rm'ing the string docs myself and comparing his patch against the result):
 1. Dos line endings
 2. No newline at the end of the file

His patch applies cleanly when I fix those (using "fromdos" tool to fix (1), and opening in emacs & inserting newline at the end to fix (2)).

Given that this is just a removal of a few HTML documentation files, this should be safe to push as a one-off DONTBUILD cset, so I'll just do that since I've already fixed it up.
Comment 11 Daniel Holbert [:dholbert] 2011-05-06 12:46:26 PDT

Tyler, for future patches, if you could address comment 9 & also fix whatever caused the whitespace issues in comment 10, that'd be much appreciated.  Thanks!

Note You need to log in before you can comment on or make changes to this bug.