Bug 632594 created; review, approval, commit requested
janssens-geert at telenet.be
Wed Oct 20 09:25:55 EDT 2010
On Wednesday 20 October 2010, Thomas Bullock wrote:
> I reviewed your links and have these questions.
> 1. Are you saying do not modify gnucash-guide.xml at this time? If so, is
> that because Cristian is working on this module?
You are free to modify gnucash-guide.xml. Especially if your changes are
small. When I told you about using 'svn update' regularly to pull in changes
made by other contributors, that is equally valid for Cristian. Before he
commits his changes, he'll probably first run an svn update as well to be sure
all changes made by others are in his working copy. If necessary he may have
to tweak some of those external changes to align with his local changes. But
that is normal in a collaborative environment. It would be more complicated if
two people were making big structural changes at the same time in the same
file. In that case you better coordinate more closely.
On the other hand, your patch as seen in bugzilla doesn't contain
modifications to gnucash-guide.xml, so I wonder how you interpreted Yarar's
reply as if you are not allowed to change that file.
> If I don't modify it,
> then I just drop it from what my 'svn diff' found?
If your diff contains changes you don't want to propagate (yet), you can do
this indeed, but this particular method is error prone. A better way is to
revert the changes from the original file (possibly saving a copy temporarily
if you want to keep the changes for a later time) and recreate the patch with
> I thought I had to
> correct all of 'xmllint' and 'svn diff''s findings?
xmllint reports on xml errors. svn diff simply shows you exactly what you have
changed. In that context, "correcting svn diff's findings" doesn't seem to
make much sense to me. svn diff will not judge if the changes are right or
wrong, so it will not give you anything to correct per se. As just mentioned,
if svn diff reports changes you don't like, the preferred method is to revert
those changes in your original file and run svn diff again.
> Cristian Marchi told
> me to fix them when I found them. See his email dated 10/19/2010, listed
> as the #3 in the digest vol 91, Issue 26. I don't know what you mean by
> mean fixing line breaks.
xmllint errors should be fixed indeed.
Just to understand what is happening here, I have checked out r19679, which is
the last revision just before your patch is applied by Yawar. This revision
doesn't hold any xmllint errors anymore as far as I can see.
At the same time, I don't see anything in your patch that would fix an xmllint
I can only assume this is what happened:
1. you had fixed a number of xmllint errors in your local working copy.
2. at about the same time, Cristian committed his work in r19679, which among
others also fixes the same xmllint errors.
3. you dutifully ran svn update before creating your final patch. This svn
update pulled in the final xmllint fixes that Cristian had just committed.
Since your local fixes were the same as in the online svn repo, svn update
marked these local changes as in sync with the online repo. In other words,
your local fixes wouldn't appear in the subsequent svn diff, because they were
already in the online repository.
4. You ran svn diff to create the patch, still assuming there would be xmllint
fixes in it.
But because these fixes are already in the online repo, what is basically left
in your patch is the tense correction, the addition of the new chapters (more
on that in a minute) and some wrapped lines.
It's this wrapping of lines that Yawar asked to postpone to a later date, when
the documentation updates have stabilized a little more.
> 2. Regarding removing payroll chapter 14 by accident, please explain why
> that is? Before committing the patch I transformed the xml files to html
> and examined chapter 1 for the overview. It showed me chapters 11 thru 16
> without any chapters missing. I looked at your changesets 19684 and 19686
> and both seem to me to be saying that I am not deleting chapter 14, but
> am inserting 15 and 16 after it. In what way am I reading that
The overview will always show continuous numbering even if you delete a
chapter. The chapter numbering is dynamic. At best you could see the last
number has gone missing.
But if you would look for a chapter on Payroll in your generated html, it
would have gone missing, chapter 14 would now be the budgets chapter.
Yawar is correct, that is the change your patch is making even though you may
not have intended this:
- <para><xref linkend="chapter_bus_pay"></xref></para>
+ <para><xref linkend="chapter_budgets"></xref></para>
The link to chapter_bus_pay in the original file is being replaced (-) with a
link to chapter_budgets (+).
You could check in your original xml file if you have still linkends for both
chapter_bus_pay and chapter_budgets. The patch suggests you don't.
Of course, if you ran an svn update in the meantime, things may have changed
once more because you then have pulled in what Yawar committed, which is a
corrected version of your patch.
By the way, if you run svn status, do any of your files have a "C" state ?
Those should always be fixed before making a patch. Read up on conflict
resolution in the subversion book if you need help with that .
More information about the gnucash-devel