[sword-devel] Huge commits in SVN+ possible bug in 1.8.0

[Troy A. Griffitts]

Hey Jack,

Yes, I agree with you and usually try to keep commits logically distinct. This was one of those that kept spidering out. Initially, I was trying to fix the default CSS we provide-- specifically in the case of tenseChange, and found other CSS that was wrong... Ok, this commit can expand to be a default CSS logical change... And when looking at the default CSS in the other filters, I found initialization errors which might prevent the CSS from being correct... and then the <lg> formatting which wasn't exactly a CSS change but still a formatting change... It all should have been split out, but I didn't foresee it growing as large as it did. 

Regarding the <div></div> for lg, an empty div usually is rendered with no height but does induce a line break, by default. I didn't want to force additional vertical whitespace, in the event there was already a new line. Specifically in this case, this pushes a <div></div> into the preverse material if the verse starts with lg or l. This cleans up printing a verse number and then immediately a new line after the verse number, which doesn't look nice. 

My apologies again for the large commit. I agree, small commits are the goal,

Troy

On December 20, 2017 5:42:17 PM MST, Jaak Ristioja <jaak at ristioja.ee> wrote:
>Hello!
>
>The last commit to SVN trunk (3547) on December 10 was a large one. The
>commit message stated 7 different changes, one of which was "Fixed a
>ton
>of initialization bugs in filters". Really nice bug finds! But it was a
>terrible commit to try to understand. While trying to keep Sword++ in
>sync with Sword SVN trunk, I ended up splitting that one commit into
>about 20 different logical changes. Because I needed to grasp all the
>separate changes it introduced, I think it was the most sensible thing
>to do, given my limited intellectual abilities.
>
>Doing so, I think I also found a bug accidentally introduced by the
>commit into OSISXHTML::MyUserData::outputNewline(), which instead of
>appending a newline to the preverse, appends "<div></div>", causing
>tests to fail. You might want to take a look, because it seems to have
>made it into the sword-1-8-0 tag.
>
>Please try to avoid such mega-commits, if possible. Such are extremely
>difficult to review, and scare people away. Try to "Commit early,
>commit
>often", splitting your work into small logical changes which are easier
>to reason about.  Logical commits in version control history also help
>review of source code at a much later time, e.g. when developers dig
>deep into the history to find some answers, having to reason their way
>through such huge monoliths is usually very counter-productive.
>
>I don't know how good SVN clients are nowadays, but with git, if one
>has
>a ton of changes, one can selectively stage (for committing) a subset
>of
>them using `git add`, `git add -p` or similar, and so effectively split
>their changes into logical commits before pushing these to the central
>repository.
>
>
>Thank you and God bless!
>J
>
>_______________________________________________
>sword-devel mailing list: sword-devel at crosswire.org
>http://www.crosswire.org/mailman/listinfo/sword-devel
>Instructions to unsubscribe/change your settings at above page

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.crosswire.org/pipermail/sword-devel/attachments/20171221/5880a338/attachment.html>