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.
Thanks,
Jason
--
Jason M. Swails
_______________________________________________
AMBER-Developers mailing list
AMBER-Developers.ambermd.org
http://lists.ambermd.org/mailman/listinfo/amber-developers
Received on Wed Sep 06 2017 - 08:00:02 PDT
