Re: convert-ly rules to destencil Flags

[address@hidden]

On Oct 24, 2011, at 5:12 PM, David Kastrup wrote:

> "address@hidden" <address@hidden> writes:
> 
>> On Oct 24, 2011, at 4:29 PM, David Kastrup wrote:
>> 
>>> 
>>> Mike has pushed directly 671b7b63408893c33b4c1f196e87db19a7dbcd1e to
>>> master, as far as I can see without any discussion and without going
>>> through staging.
>>> 
>> 
>> This got to patch push after going through a countdown.
> 
> Ok, so my mistake, and the only one not following rules at all am I.
> The problem is that to do a full review of convertrules means actually
> _applying_ them and looking at the results (including testing them, but
> also checking visually).  Which does not appear to have happened here.
> 

I did this on a test file and did not see any problems.  I did not run it on 
the docs because, as you pointed out, I updated everything manually before the 
idea came to me to write this rule (which only governs a certain easy case - 
other stencil overrides are not convert-ly-able).

>> As for staging, I'm still waiting on a response for how that works so
>> I can do it.
> 
> git fetch
> git rebase origin/dev/staging
> git push origin HEAD:dev/staging
> 

Thank you.

>> However, I did a complete build with this patch and know it doesn't
>> break anything, so I don't see how staging would have helped here.
> 
> The patch does not break anything, but the output of actually _running_
> the convertrules (which is their whole point) is partly not pretty,
> partly wrong (because of double updates).
> 
>> Which files?  Were these files part of a countdown and, if so, were
>> they commented upon?  Going through the Flag patch was my first time
>> making a change like this, and it would have helped to get comments
>> from people who know how this works regarding versioning.
> 
> The problems are the convertrules, not the files that have _not_ _yet_
> been committed but which will result from running convert-ly.
> 
> I recommend you do the following: first pull from master (if you are not
> interested in seeing why I improved the rules).
> 
> Make sure you have no uncommitted changes (git diff should be empty).
> 
> Then run
> 
> scripts/auxiliar/update-with-convert-ly
> 
> When it finishes, check the output of "git diff".  You will see that
> there are files which now contain duplicate fixes.  Verify that those
> contain _only_ duplicate fixes (manual and automatic) as well as the
> version number update and write down their names.
> 
> Now do
> git reset --hard
> 
> For all of the files which were double-fixed, edit their version string
> to indicate that they are already up to date (according to your last
> convertrule).
> 
> Do a commit that contains just these files with their updated version
> string, reflecting the version they represent after your previous manual
> fixes.
> 
> Now run scripts/auxiliar/update-with-convert-ly
> 
> Check that the output of "git diff" is sane.  Run the regtests.  If they
> work out, commit.
> 
> Put out for review and/or push to dev/staging.  Since we are talking
> about fixups to something which has already passed reviews and
> countdowns, I'd consider dev/staging reasonable, and I won't be stalled
> anymore.  If Graham disagrees with that assessment, he need not pull the
> changes over into master.
> 
> Note that this procedure is only ok if the intermediate state before
> running convert-ly also does not break regtests (merely looking ugly
> does not count), or bisecting will get hampered.  Since we already
> arrived at this "intermediate" state with working regtests, this is just
> something to keep in mind next time around.
> 

Will do.

Cheers,
MS