Closed Bug 216707 Opened 18 years ago Closed 18 years ago
Patch Viewer needs user documentation
Patch Viewer has recently landed and needs user-level documentation. An HTML version of them can be found at http://www.johnkeiser.com/mozilla/Patch_Viewer.html . They need to be converted to the XML format used in Bugzilla.
OK, here they are.
Comment on attachment 130096 [details] [diff] [review] Docs >Index: xml/using.xml >=================================================================== >+ integrationg with Bonsai, LXR and CVS></para> You have a > instead of a . >+ <section id="seeing_difference_between_two_patches"> >+ <title>Seeing the Difference Between Two Patches</title> >+ <para>To see the difference between two patches, you first view the >+ newer patch in Patch Viewer, and then select the older patch from the Gramatically speaking, I don't think there's supposed to be a comma after "Viewer." I would recommend breaking this up into two different sentences, but it is up to you. >+ dropdown at the top of the page ("Differences between [dropdown] and >+ this patch") and click the "Diff" button. This will show you what Is there a reason for having parenthsis and quote marks there? >+ <section id="getting_more_context_in_a_patch"> >+ <title>Getting More Context in a Patch</title> >+ <para>To get more context in a patch, you put a number in the textbox at >+ the top of Patch Viewer ("Patch / File / [textbox]) and hit enter. There's a mismatched quote. >+ <section id="collapsing_and_expanding_sections_of_a_patch"> >+ <title>Collapsing and Expanding Sections of a Patch</title> >+ <para>To view only a certain set of files in a patch (for example, if a >+ patch is absolutely huge and you want to only review part of it at a >+ time), you can click the "(+)" and "(-)" links next to each file (to >+ expand it or collapse it). If you want to collapse all files or expand >+ all files, you can click the Collapse All and Expand All links at the For conisitancy sake, you should either put quotes around Collapse and Expand All or not around the +/-. I would also choose shorter section id's. My recommendation would be something along the lines of (I prepend "patchviewer_" to help guarntee uniqueness): patchviewer_context, patchviewer_collapsing, etc. These are mostly minor, but I think I would like to see them at least addressed before granting review :) (Again, sorry for the long timespan)
Attachment #130096 - Flags: review?(jake) → review-
This fixes all comments except "why do you have that sentence both parenthesized and quoted?" It is parenthesized because it is an aside, and it is quoted because it represents text on the screen--without the quotes it can be confused as instructions rather than text to look for.
Attachment #130096 - Attachment is obsolete: true
Comment on attachment 134502 [details] [diff] [review] Docs v1.1 Asking kiko so this might make the next release.
Attachment #134502 - Flags: review?(kiko)
Comment on attachment 134502 [details] [diff] [review] Docs v1.1 I would have written the opening paragraph starting with a positive comment, but the text is fine as it is :-)
Attachment #134502 - Flags: review?(kiko) → review+
Comment on attachment 134502 [details] [diff] [review] Docs v1.1 Comments on my wishlist, you may integrate before checking in if you like. They don't block review at all. >+ <para>There has been some difficulty viewing and reviewing patches in >+ Bugzilla due to lack of context and complexity of reading the raw >+ patches. Patch Viewer is an enhancement to Bugzilla designed to >+ fix that by offering increased context, linking to sections, and >+ integrationg with Bonsai, LXR and CVS.</para> I would rewrite the opening statement as: Viewing and reviewing patches in Bugzilla is often difficult due to lack of context, improper format and the inherent readability issues that raw patches present. >+ <simplelist> >+ <member>View patches in color, with side-by-side view rather than trying >+ to interpret the contents of the patch</member> >+ <member>See the difference between two patches</member> >+ <member>Get more context in a patch</member> >+ <member>Collapse and expand sections of a patch for easy >+ reading</member> >+ <member>Link to a particular section of a patch for discussion or >+ review</member> >+ <member>Go to Bonsai or LXR to see more context, blame, and >+ cross-references for the part of the patch you are looking at</member> >+ <member>Create a rawtext unified format diff out of any patch, no >+ matter what format it came from</member> >+ </simplelist> I would add periods to the end of the items here. >+ <section id="patchviewer_view"> >+ <title>Viewing Patches in Patch Viewer</title> >+ <para>The main way to view a patch in patch viewer is to click the >+ "Diff" link next to a patch in the Attachments list on a bug. You may Perhaps click *on* the "Diff" link >+ <section id="patchviewer_context"> >+ <title>Getting More Context in a Patch</title> >+ <para>To get more context in a patch, you put a number in the textbox at Perhaps you may put / you should put to match the other paragraphs. >+ the top of Patch Viewer ("Patch / File / [textbox]") and hit enter. (Why doesn't this have "Context:" or "C:" labelled in the UI? :-) >+ This will give you that many lines of context before and after each >+ change. Alternatively, you can click the "File" link there and it Perhaps click on the "File" link >+ <section id="patchviewer_link"> >+ <title>Linking to a Section of a Patch</title> >+ <para>To link to a section of a patch, for example if you want to be >+ able to give someone a URL to show them which part you are talking >+ about, you simply click the "Link Here" link on the section header. The You used a parenthesis to denote your example in the last section; maybe it makes sense to use the same notation here. This is perhaps my greatest überübernit of all times <wink> >+ <section id="patchviewer_bonsai_lxr"> >+ <title>Going to Bonsai and LXR</title> >+ <para>To go to Bonsai to get blame for the lines you are interested in, >+ you can click the "Lines XX-YY" link on the section header you are >+ interested in. This works even if the patch is against an old >+ version of the file since Bonsai stores all versions of the file.</para> I would add a comma before Bonsai there.
This is the patch I just committed.
Attachment #134502 - Attachment is obsolete: true
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.