On Wed, Sep 11, 2013 at 9:38 PM, Ben Reser <ben_at_reser.org> wrote:
> On 9/1/13 2:42 AM, Johan Corveleyn wrote:
>> That's not really an argument, because that's true whether you do it
>> at commit time or afterwards. You don't save any time (quite the
>> contrary) by postponing it until CHANGES needs to be produced, or
>> until the revision needs to be backported. Except perhaps by having
>> better understanding of which changes matter to users and why, but I'd
>> say that's true for perhaps 5% of CHANGES.
>>
>> Unless you're saying you don't want to put this burden on the
>> committer (right at the point of committing), because that would
>> hinder commits? Then I'd argue that this is just a habit, and once you
>> get used to it, it doesn't really slow you down much (just like we are
>> all used to writing good, concise, high-information-density log
>> messages for our fellow developers). And for those really hard ones,
>> you can always come back to the log message later, and massage the
>> user-facing text (or add it if it's not there).
>>
>> So that would leave the argument of "that's not what we are used to
>> do", and I can accept that (habits matter, and can be hard to change).
>> It's an important argument, but not a technical one.
>>
>> So far, I haven't read a really rational argument against my
>> suggestion. "That's not what log message are for"? Why not?
>
> I think I already replied giving those reasons which you reply to further on down.
>
> It essentially comes down to whatever we put in commit messages on trunk won't
> necessarily be accurate when we go to produce a new minor release (as in
> major.minor.patch)
>
>> Fair enough. I understand you're currently only trying to solve the
>> CHANGES-for-backports problem for the RM, and that's okay.
>>
>> I just want to put this in the larger context of producing user-facing
>> messages for our changes for *all* our releases, minor or major. Think
>> back to all the time spent by several developers while preparing for
>> the 1.8.0 release, sifting through all the log messages since the last
>> release (in batches of 500 commits), trying to extract user-facing
>> information [1]. That's a huge waste of valuable developer-time, IMO.
>
> If there really is a better solution I'm open to it, I'm just not convinced
> that there is a good solution for trunk -> minor release.
>
> There are so many problems with automating it that I think it'll take a lot of
> time to sort them all out and for all pracitcal purposes minor releases are
> relatively rare.
>
> If we start doing them more often then a lot of the problems with automating it
> go away because there's less likelihood that the Changes: line in the commit is
> out of date by the time we do the minor release.
>
>> I agree, making concurrent commits to CHANGES is a bad idea. Much
>> better to put it in the log message :-).
>
> Realize I'm not opposed to putting better information in our commit messages to
> help produce the CHANGES later, I just don't think we can automate the
> production in a very useful way.
>
> I do think we can put a Change: line in, but I still think someone is going to
> have to review all of them for reasons I'll get into below.
>
>> Ah, but that can be solved perfectly automatically (we do this at my
>> workplace too): it's the perfect use case for 'svn mergeinfo
>> --show-revs eligible'.
>>
>> When you're about to produce CHANGES for 1.9.0, and 1.8.6 is currently
>> the latest 1.8 release, you'll want to process all the revisions
>> produced by
>>
>> svn mergeinfo --show-revs eligible ^/branches/1.9.x ^/tags/1.8.6
>
> This doesn't completely work, sometimes we make changes that only apply to a
> given branch due to trunk having a completely different fix that can't be
> backported.
Actually, it works perfectly. Note that I requested the eligibles
between the "to-be-released-branch" and the
"previously-released-tag-of-which-we-don't-want-to-repeat-changes".
If you apply rN to ^/branches/1.9.x, that revision will show up in the
above query.
I don't have our script handy, but with some simple operations, it
works fine for collecting the commit messages of:
- revisions applied directly to the new branch
- revisions backported to the branch (and not to the previous tag)
- revisions from the natural history of the branch (and not part of
the previous tag)
>> Okay, but usually the developer who makes such a change knows he's
>> fixing something (on trunk or whatever) that's never been in the hands
>> of users. In that case, of course he doesn't write such a user-facing
>> note in the log message. The entire idea hinges on the fact that a
>> developer usually knows what his change means to users.
>
> My concern here is that people will forget to put them in when they should. If
> we automate it then there's no chance to catch this.
Okay, but then I say: this can be caught by regular post-commit
review. Similar to how we catch other omissions to the log message.
Occasionally something will be missed, but with good post-commit
review this will be rare. With the current process, with one single
person scanning hundreds of commit messages, there will also be
occasional mistakes / omissions.
>> Also, when working with feature branches, it's clear that commits to
>> the branch are usually not interesting to users. In this case you
>> usually end up with one summary "change", when the branch gets
>> reintegrate (e.g. "Change: add new FSX repository back-end
>> (experimental)", as a result of integrating the fsx branch).
>>
>>> If you look at the 411 Content-Length issue I think any attempt to put the
>>> CHANGES entry in the log files would have been a mess.
>>
>> You'll have to refresh my memory here. But there can be exceptions of
>> course, where it's better to wait a bit for the dust to settle on a
>> particular issue, and then come up with what it means to users. But
>> those are exceptions.
>
> We went back and forth between several different implementations of the fix.
> With entirely different default behavior.
>
> The other issue here is how do you remove a CHANGES entry that's no longer
> important? The only way I can see if you put it in commit logs is to go back
> and edit the commit log to remove the Changes: line. But I'm not convinced
> that we will actually do that.
Hm, you may have a point there. With the weekly release cycle at our
company such situations are quite rare. Still, sometimes we end up
with conflicting or overlapping notes (not often, but it happens).
Like:
- (issue #1234) Change color of SomeForm to have more contrast.
- (issue #1234) Rollback color change to SomeForm. Testers pointed out
potential problems for color-blind people.
Or:
- (issue #1234) Change color of SomeForm to yellow.
- (issue #1234) Change color of SomeForm to green.
- (issue #1234) Make color of SomeForm user-configurable.
>> Also, most commits won't end up with a "Change:" note, just like not
>> every commit has a "Review by:" note. Only when you have something
>> interesting to say to users.
>
> Sure I understand that. But that's exactly the problem that complicates this.
> Those attribution tags get missed all the time as well. The difference though
> is that the person who should have been attributed can say something or fix it
> themselves.
>
> If it was something we needed on every commit it's much easier to review for
> that. But when it may be appropriate to ommit it then it's unlikely someone is
> going to point out they are missing because then they'd have to go through the
> process of figuring out if it was necessary.
>
> Maybe the alternative is to require some sort of Change line in every commit
> and to specify why one isn't necessary somehow when that's the case. But I
> suspect people will object to that because it's just more tedious work to put
> into a commit.
Indeed, it's not intended to be added to every commit, that would be
tedious. Like I said, I think post-commit review would have to ensure
that no things are missed.
But I agree that "evolving changes" and "changes being rolled back",
without going back to edit the outdated "change lines", could be a
significant problem (especially with long release cycles). So I won't
push this any further :-), and leave it at that.
--
Johan
Received on 2013-09-20 01:10:28 CEST