Re: [BuildStream] Some bad mistakes, lets understand them, and make this work better



Hi,

On Thu, Sep 13, 2018 at 1:43 PM Tristan Van Berkom <tristan vanberkom codethink co uk> wrote:

[...]
> > I very strongly agree with you and would prefer a tighter circle of
> > committers myself.
> >
> > I'm not sure how we can really describe that in terms of policy,
> > though, and it was raised to me that we probably need to be acting on
> > policy.
> >
> > I can see that side of the coin, even though I personally have greater
> > confidence in a completely ad hoc approach where people use critical
> > thinking to decide to hand out commit access on a case by case basis
> > (it wouldn't be perceived as "fair", but I'm not sure it needs to be).
>
> The process can be subjective.  I'll refer back to http://subversion.apache.org/docs/community-guide/roles.html#committers as an example of something that works.

This looks very good to me.

If there are no strong objections, I would very much like to implement
this model for BuildStream.

I do feel that we might be adding too much process for a project which
is not really quite that huge in scale, that said; we already have more
people working on the project than I ever anticipated.

Perhaps I should raise this as a separate proposal to the list.

I think it definitely deserves its own thread :).
 
[...]
> Do we want to do something at the MR level that provides guidance like:
> http://subversion.apache.org/docs/community-guide/conventions.html#log-messages
>
> Or do we want to do that on a per commit basis?  Or not at all?

I think we want it on a per commit basis.

And I think that the subversion example in this case is overkill, but
agree that we could better than:

  http://buildstream.gitlab.io/buildstream/HACKING.html#commit-messages

It's frobilicious!

In the text above the linked section, we do mention:

 "If the branch fixes an issue or is related to any issues, these
  issues must be mentioned in the merge request or preferably the
  commit messages themselves."

But we should just be less ambiguous and more strict about that, just
always mention the issue in the commit message is better than leaving
wiggle room.

I think that linking to issues is one thing.  The other has more to do with describing the code change.  When I was working on my first patch to subversion I remember thinking it was very administrative.  However, when writing the log message like that, it forces you to look at all the functions you touched, what you changed and why.  When it doesn't match the summary of "The new frobnicator frobnicates foos all the way throughout the element. Elements that are not properly frobnicated raise an error to inform the user of invalid frobnication rules." you have a flag for yourself.  It also helps with review, because without reading the actual diff you can already ask "we're doing what now?"
The side effect is that the code changes are all documented with the code base, without requiring the issue tracker to persist indefinitely.  It's also great for people new to the project to go through history and see how changes are constructed.

As someone who isn't actively submitting changes, I'll defer to those of you that are, to make a call on whether this will help the project or not.
 
[...]
> > Of course, it's significant churn and costs some effort, feature
> > branches would conflict a lot; but the refactor would be rather
> > mechanical and straight forward to carry out (albeit still a lot of
> > work).
> >
> > Perhaps this change is worth it for the added linting securities it can
> > provide.
>
> It feels like an extreme measure, and potentially unnecessary if
> people know what the expectations/rules are?  

I'm actually unsure if this is </sarcasm> :)

That one actually wasn't  :).

[...]

Cheers,

Sander
 
Cheers,
    -Tristan

--

Cheers,

Sander


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]