On Wed, Sep 6, 2017 at 10:30 AM, Jason Swails <jason.swails.gmail.com>
wrote:
> On Wed, Sep 6, 2017 at 9:12 AM, Hai Nguyen <nhai.qn.gmail.com> wrote:
>
> > Per broken commit: I think the typical workflow is: if the commit broke
> the
> > test case or broke the build, we should revert the commit immediately and
> > let the author fix that in different branch and then merge back to master
> > if all are ok. It's really hard to fix things after a tons of commit. So
> > hopefully no one feels upset if your commit was reverted.
> >
>
> To me this is like using a sledgehammer to drive everything from a tent
> stake to a thumbtack. Handling the highly asynchronous nature of the
> commit-push-test process in a way that doesn't cause problems much worse
> than the original makes automating this process a challenge. So the
> process will likely need to be human-triggered. Consider a very common
> simple case of someone forgetting to add a file in a commit. If the
> reversion happens, whoever made the mistake would need to do a "git pull",
> which would wipe out all the changes they made and they'd need experience
> with git (or know what to google) to revert the reversion, add the fix, and
> push it up to the Amber git repo. And that's if they didn't make any more
> changes they haven't pushed, which would make things even more
> complicated. Compare this with just adding the file and pushing it.
>
> And people *will* feel upset or discouraged if their commit is reverted,
> especially new developers who will be very concerned about making a mistake
> (I still remember how I felt when I pushed my first commit). If one or
> more of my first few commits had been immediately reverted with an
> admonishment of "you broke stuff," I likely would have been far more wary
> of committing again in the future. Confidence and efficiency is built over
> time, and a development model with high turnover like Amber employs
> absolutely depends on new developers rising up to replace those that leave
> (to go work on lighting control, for instance).
>
> I understand the annoyance an experienced developer experiences when master
> is broken even for half a day -- it's happened to me many times. But
> absent a workflow based on pull requests and merge gates, it's just
> something you have to deal with. In my experience, master rarely stays
> broken that long, and people responsible for breaking have always been
> responsive when it comes to fixing their mistake.
>
> My suggestion is to move from gitosis to a tool that implements a
> PR/CI-gating workflow like GitLab (which can be self-hosted). Disable
> pushing directly to master and make every change pass through a gated pull
> request that enforces some level of quality before merging is permitted.
> Until you have something like that in place, what we're currently doing is
> probably the best possible workflow.
>
> So my vote is to stop calls for reverting everything that breaks the build,
> and work toward building the infrastructure needed to minimize the
> likelihood that breaks creep into master in the first place as well as
> minimize the time required to detect the break and fix it. Reverting a
> change should be the last tool used to address an issue, not the backstop
> that catches everything.
>
> Just my thoughts.
>
>
good point, Jason.
Hai
_______________________________________________
AMBER-Developers mailing list
AMBER-Developers.ambermd.org
http://lists.ambermd.org/mailman/listinfo/amber-developers
Received on Wed Sep 06 2017 - 08:00:04 PDT
