GNOME Bugzilla – Bug 670766
Add task-oriented Mallard help
Last modified: 2016-03-31 13:22:07 UTC
As discussed with Marc-Andre Lureau and Baptiste Mille-Mathias with the input of the Documentation Team.
*** Bug 686779 has been marked as a duplicate of this bug. ***
Working on the documentation in the 'mallard-help' branch in the git.gnome.org repository. Contributions welcome.
(In reply to comment #2) > Working on the documentation in the 'mallard-help' branch in the git.gnome.org > repository. Contributions welcome. Cool, soon we'll have docs! I suggest putting the build system changes and planned sections/pages first. Once the patch(s) to add those are submitted here, we can work together on different pages then. Pushing directly to a branch rather than submitting patches here to be pushed one by one directly to git master, is fine but that would mean we can't review the docs/patches on the way but rather only at the end.
Created attachment 238107 [details] [review] [doc] Adding first stub pages
Created attachment 238108 [details] [review] Add supported protocols
Created attachment 238109 [details] [review] Add more content to protocol page
Created attachment 238110 [details] [review] Add page system-requirements.page
Created attachment 238111 [details] [review] Add more content to system-requirements.page
Created attachment 238114 [details] [review] Help merge: the other sixteen git-bz started giving me a '500 Internal Server Error', so I switched to git format-patch.
Comment on attachment 238111 [details] [review] Add more content to system-requirements.page This commit is in help.patch
Created attachment 238117 [details] [review] patch
Created attachment 238120 [details] [review] fixed whitespace I have deleted the mallard-help branch as teuf requested so that autocomplete works for master from 'ma' and pushed the help to wip/user-help
Comment on attachment 238120 [details] [review] fixed whitespace I have pushed all these patches now. I'll fix a few small things now and then we need to add a Help menu item (which would require a freeze break request). For future references please keep 1 file for each patch. I see a lot of patches that could have been simply squashed into existing (unmerged) patches as they were simply fixing them. Kat mentioned at docs hackfest that issue with squashing patches is that you loose authorship of the correcting patches. This problem will not arise if the usual method of development is followed: Patch first comes to bugzilla for review, Instead of someone correcting in a separate patch, that person points out the issues/problems in the review and the original author then fixes the patch ('-e' option of git-bz attach is very handy for resubmittin same patch). Having said that, you guys are doing a great job so don't let my pedantic nature discourage you in any way. I will take good contributions anyways but if you act on the suggestions above, it will make it much easier for us to work together.
(In reply to comment #13) > For future references please keep 1 file for each patch. I see a lot of patches > that could have been simply squashed into existing (unmerged) patches as they > were simply fixing them. Slightly off topic: could you add a HACKING file to the project that details your preferred workflow?
Created attachment 238128 [details] [review] help: Improve system requirements page
(In reply to comment #14) > (In reply to comment #13) > > For future references please keep 1 file for each patch. I see a lot of patches > > that could have been simply squashed into existing (unmerged) patches as they > > were simply fixing them. > > Slightly off topic: could you add a HACKING file to the project that details > your preferred workflow? Sure, I should look into that. Although what I described in comment#13 is mostly it and its the working flow for many projects. Most (if not all) projects outside GNOME use mailing-list rather than bugzilla for patches but otherwise they follow the same work flow.
Review of attachment 238128 [details] [review]: Seems OK to me, but best check the style with someone on the docs team.
Attachment 238128 [details] pushed as 9d880d7 - help: Improve system requirements page
Accidently closed this.
Review of attachment 238128 [details] [review]: The page does not validate, see review comments for the rest. ::: help/C/system-requirements.page @@ +22,3 @@ + <p>To create and run virtual machines in <app>Boxes</app>, your computer + should meet certain requirements:</p> "Must", not "should" if they are requirements. If they are recommended criteria, the sentence needs to be reworded. @@ +30,3 @@ </list> + <p>The following are highly recommended (but not required) for decent Do not qualify "recommended". Do not use brackets. Use "better", not decent. @@ +33,3 @@ + performance of virtual machines:</p> + <list> + <comment><p>have a page to check cpu extensions</p></comment> Comments cannot be inside lists. @@ +42,3 @@ <note> + <p><app>Boxes</app> should be able to create and (efficiently) run + virtual machines in any computer sold since 2010.</p> This is incorrect: delete the paragraph.
Created attachment 238147 [details] [review] Stub connect.page
Review of attachment 238128 [details] [review]: This patch is committed. So I'll have to fix things in a new patch. ::: help/C/system-requirements.page @@ +30,3 @@ </list> + <p>The following are highly recommended (but not required) for decent What should I write instead of "recommended" here? "better" would require a "than X" part, no? Do I delete everything inside brackets or take it out of brackets? @@ +42,3 @@ <note> + <p><app>Boxes</app> should be able to create and (efficiently) run + virtual machines in any computer sold since 2010.</p> Its just a modification of what was already there.
Created attachment 238148 [details] [review] Review search.page
Created attachment 238153 [details] [review] patch (In reply to comment #22) > This patch is committed. So I'll have to fix things in a new patch. See attached patch. > @@ +42,3 @@ > <note> > + <p><app>Boxes</app> should be able to create and (efficiently) run > + virtual machines in any computer sold since 2010.</p> > > Its just a modification of what was already there. That doesn't change the fact that this is incorrect :) Incorrect statements need to be removed, not modified.
Review of attachment 238147 [details] [review]: Personally, I would rather we add some content instead but up to you.
Created attachment 238154 [details] [review] CC-by-SA licence added after acquiring confirmation from all authors.
Comment on attachment 238147 [details] [review] Stub connect.page Pushed to master in commit ac7164fa9170379dfe0a7d266e5a396c0c6d9666
Review of attachment 238148 [details] [review]: Seems like 2 separate changes (license and corrections) so better put in separate patches. Also suggest a better short log: Improve/correct search help page. ::: help/C/search.page @@ +31,1 @@ + <p>You can search for a box by name <link xref="collection">collection "by name" -> "by name in" @@ +31,2 @@ + <p>You can search for a box by name <link xref="collection">collection + view</link> or <link xref="selection">selection mode</link> when you have selection-mode is a mode of collection view and this para seems to give the wrong impression.
Review of attachment 238153 [details] [review]: Looks good otherwise. ::: help/C/system-requirements.page @@ +53,3 @@ + </item> + <item> + <p>The <sys>kvm</sys> module loaded in the <em>Linux kernel</em>.</p> We should specify that this is something generally not needing any user intervention and is taken care of by the system (libvirt pacakges).
Review of attachment 238154 [details] [review]: Looks good
Created attachment 239082 [details] [review] Unstubbed create.page, added content, against wip/user-help branch
(In reply to comment #31) > Created an attachment (id=239082) [details] [review] > Unstubbed create.page, added content A number of items in this page will link to explanatory content in other pages.
Comment on attachment 238148 [details] [review] Review search.page This was obsoleted IIRC. Feel free to correct me.
Created attachment 239672 [details] [review] Unstubbed pages and added content
Review of attachment 239672 [details] [review]: Looks good otherwise. ::: help/C/connect.page @@ +19,3 @@ + <p>When <app>Boxes</app> is launched, the collection view opens, displaying + your existing boxes. If you haven't yet created a box, the greeting reads + <gui>No boxes found.</gui></p> How come this is under "Connect to another computer" title? I'm not sure if this even belong to this page. @@ +40,3 @@ + <item><p>spice:// to connect to a remote <app>Xspice</app> + server.</p></item> + <item><p>qemu:// to connect to a box or qemu instance on a remote * remote -> local or remote * qemu instance -> libvirt instance providing access to all virtual machines it is hosting ::: help/C/create.page @@ +38,3 @@ + + <list> + <item><p>select an iso from the list gathered by Tracker.</p></item> Don't think we need to mention "Tracker" here? I think it should be user-invisible. We could simply say something like "select one of the automatically picked up ISOs from the list". Probably should also add a note here: If your ISOs are not automatically picked up by Boxes, try putting them in some standard location such as 'Downloads' folder under your home folder.
(In reply to comment #35) > Review of attachment 239672 [details] [review]: > > Looks good otherwise. Fixed and pushed. > automatically picked up ISOs from the list". Probably should also add a note > here: If your ISOs are not automatically picked up by Boxes, try putting them > in some standard location such as 'Downloads' folder under your home folder. This can be added as a separate page under Problems.
Comment on attachment 239672 [details] [review] Unstubbed pages and added content Oops, wrong status.
(In reply to comment #26) > Created an attachment (id=238154) [details] [review] > CC-by-SA licence added after acquiring confirmation from all authors. Pushed to wip/user-help.
(In reply to comment #29) > We should specify that this is something generally not needing any user > intervention and is taken care of by the system (libvirt pacakges). Is the kvm module loaded by libvirt? Or is this something that is related to packaging?
(In reply to comment #39) > (In reply to comment #29) > > We should specify that this is something generally not needing any user > > intervention and is taken care of by the system (libvirt pacakges). > > Is the kvm module loaded by libvirt? Or is this something that is related to > packaging? Kvm module is automatically loaded by libvirt, yes if virtualization extensions are not disabled in BIOS. Daniel can confirm this so putting him on CC.
(In reply to comment #40) > Kvm module is automatically loaded by libvirt, yes if virtualization extensions > are not disabled in BIOS. Daniel can confirm this so putting him on CC. Is it possible for libvirt to not load the kvm module when virtualization extensions are enabled? If yes, under what conditions?
(In reply to comment #41) > (In reply to comment #40) > > Kvm module is automatically loaded by libvirt, yes if virtualization extensions > > are not disabled in BIOS. Daniel can confirm this so putting him on CC. > > Is it possible for libvirt to not load the kvm module when virtualization > extensions are enabled? If yes, under what conditions? No, that's wrong. libvirt does *not* load the KVM kernel module. The OS vendor's packages for KVM typically setup rules such that the KVM module is loaded at bootup. eg on Fedora the qemu-system-x86 RPM adds /etc/sysconfig/modules/kvm.modules to load KVM on boot.
(In reply to comment #40) > yes if virtualization extensions are not disabled in BIOS. Emphasis on this part, as it's pretty common for this to be disabled in BIOS by default. In this case the kvm modules fail to load, and there's a message in dmesg.
(In reply to comment #29) > We should specify that this is something generally not needing any user > intervention and is taken care of by the system (libvirt pacakges). Whether the KVM module is loaded or not appears to be distribution specific, so we cannot assume that it will be loaded. Therefore, it should specified that the module needs to be loaded for optimum performance. We cannot tell the user that it should be loaded automatically as we cannot know what distribution they are using and what that distribution does in the packaging… unless you can check that every distribution attempts to load it automatically.
Created attachment 241456 [details] [review] Review-system-requirements-help-page
(In reply to comment #44) > (In reply to comment #29) > > We should specify that this is something generally not needing any user > > intervention and is taken care of by the system (libvirt pacakges). > > Whether the KVM module is loaded or not appears to be distribution specific, Its not exactly distribution-specific. It they are shipping Boxes, its really a bug if they don't ensure kvm is automatically loaded if VT is enabled in the BIOS. They should at least ensure KVM get loaded if user installs Boxes. >We cannot tell the user > that it should be loaded automatically Actually we shouldn't tell anything to user about this, which is what I originally suggested.
Review of attachment 241456 [details] [review]: In the commit log, you are mentioning changes w.r.t an older version of this patch that hasn't been committed. Those should be mentioned in the bugzilla comment but not the patch's log itself. Commit log should mention whats being done by the patch itself (including the summar/shortlog). ::: help/C/system-requirements.page @@ +35,3 @@ <list> + <item> + <p>At least 8 GB of storage space per virtual machine.</p> 5 actually and even that is dependent on the recommended/minimum disk space specified for the OS by its vendor. 5G is used when we don't have this info. @@ +38,3 @@ + </item> + <item> + <p>At least 1 GB of RAM per active virtual machine.</p> This is also dependent on minimum/recommended from vendor. @@ +47,3 @@ + <note style="advanced"> + <p><app>Boxes</app> performs best if your computer processor supports + <em>hardware virtualisation extensions</em> and if these are enabled.</p> Should probably tell where they need to be enabled?. Probably there is a more friendlier name for BIOS setup?
Created attachment 241588 [details] [review] help: More info on hardware requirements Co-author: Zeeshan Ali (Khattak) <zeeshanak@gnome.org>
Attachment 241588 [details] pushed as d6eb6e2 - help: More info on hardware requirements Feel free to reopen this bug if you think my changes to the patch needs improvement.
Created attachment 241604 [details] [review] help-Improve-language-in-system-requirements
Created attachment 241606 [details] [review] help-Improve-language-in-system-requirements Sorry, attached the wrong patch!
Review of attachment 241606 [details] [review]: Looks good otherwise. 10 points for a nice commit log. :) ::: help/C/system-requirements.page @@ +36,3 @@ + creating and running virtual machines in <app>Boxes</app>. These criteria + vary depending on the operating system for the virtual machine. If the + the criteria are not specified, the following defaults are used:</p> while this feels an improvement over what I wrote, one comment: "criteria are not specified" by who? Wouldn't this leave the user wondering and think that he/she needs to specify? I would like to mention here is that Boxes tries allocate the right resources automatically. @@ +53,3 @@ <note style="advanced"> <p><app>Boxes</app> performs best if your computer processor supports + <em>hardware virtualisation extensions</em> and if these are enabled.</p> I'm fine with removing the link but I still insist on giving some clue to user where these need to be enabled. While ideally user should not hear of BIOS *ever*, in this case it seems inevitable. This will only lead to lots of users not knowing why their VMs are terribly slow.
Review of attachment 241606 [details] [review]: ::: help/C/system-requirements.page @@ +53,3 @@ <note style="advanced"> <p><app>Boxes</app> performs best if your computer processor supports + <em>hardware virtualisation extensions</em> and if these are enabled.</p> Oh I came to think of a nice compromise here between our opinions: How about we add the info about BIOS to advanced section as you suggested but provide a link here?