[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: updating merge request
From: |
Lukas-Fabian Moser |
Subject: |
Re: updating merge request |
Date: |
Fri, 20 Jan 2023 08:05:46 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 |
Am 20.01.23 um 07:38 schrieb Aaron Hill:
On 2023-01-19 8:54 pm, David Zelinsky wrote:
Am I misunderstanding something? Or is what it says in 3.2 a mistake?
If GitLab works similar to GitHub (which I have more experience with),
any new commits to your branch should automatically be picked up as
part of the open merge/pull request. When accepting the request,
there is an option to rebase or squash, although some project owners
will ask that the original submitter manually do this before
accepting, but again this is all being done in the same branch of the
pending request.
I think 3.2 might only be advising against creating a *new* branch and
submitting another request. But the --amend instruction seems odd as
that is usually about correcting an incomplete commit, for instance
when a file that was supposed to be staged was missed. It looks clunky
to have a second commit when you can often get away with fixing up the
original commit to be complete. Otherwise, you are basically rewriting
history, and that is a general no-no. One should let all commits sit
clearly in the open during review and then at the end squash or rebase
or whatever before merging based on how one prefers the main line to
look.
Accepted practice in lilypond development is different. We all rewrite
history in our local brands for the merge requests frequently and use
push --force(-with-lease). This way, the merge request is always "up to
date" without needing squashing. Gitlab itself has the feature of
keeping track of the various versions of a merge requests and letting us
browse them conveniently. This way, we never have a state of a merge
request that contains commits of the form
- feature X
- correct typo in feature X
- fix stupid mistake in feature X
- revert to previous state
etc., but (rewriting history) different versions of a merge request that
always contains
- feature X [version 1, 2, 3, ...]
So, David: git commit --amend and force-pushing are fine.
(Personally, I use two local branches for developing a feature: one in
which I keep track of my progress by adding commit after commit, and one
which is the "squashed" version that I update with amend/squash and use
for pushing. git's interactive rebase feature is my best friend here.
But there might be better methods.)
Note that it's not strictly necessary that a merge request consists of
one commit only: Logically independent "smallish" commits are fine
(possible example: one commit for the feature, one for the larger
documentation revision the feature necessitates). But always make sure
that each single commit gives a "working" version of LilyPond in order
to allow for bisecting later, which necessarily steps to "arbitrary"
commits in the history. Example for that:
commit 23cfed4c16770d85f4709275f1c78ff138c37262
Author: Jean Abou Samra <jean@abou-samra.fr>
Date: Wed Jan 4 22:10:26 2023 +0100
Assume all colors have a transparency value in output backends
The previous commit makes (color ...) stencil expressions always
contain
colors with a transparency value (4 elements, not 3). Therefore,
interpret_stencil_expression can stop differentiating between the
3-element and 4-element cases, as well as all three output backends.
commit 4e3ad954d0a20d302045afe8fee856f77a75667d
Author: Jean Abou Samra <jean@abou-samra.fr>
Date: Wed Jan 4 22:03:26 2023 +0100
Refactor colors
Add a normalize-color Scheme function that takes a color in any style
(color list, CSS) and returns an (r g b a) list. Reimplement
ly:stencil-in-color and stencil-with-color in terms of normalize-color.
Having normalize-color will enable background handling for PNG
images in
the PS backend, where the 'pngfile stencil expression will need to
include a color.
The code is moved to Scheme because I got tired of doing complicated
type checks in C++.
(I didn't check if both commits actually come from the same merge
request, but they absolutely might.)
Lukas
- updating merge request, David Zelinsky, 2023/01/19
- Re: updating merge request, Aaron Hill, 2023/01/20
- Re: updating merge request, Werner LEMBERG, 2023/01/20
- Re: updating merge request, Aaron Hill, 2023/01/20
- Re: updating merge request, Jean Abou Samra, 2023/01/20
- Re: updating merge request, Aaron Hill, 2023/01/20
- Re: updating merge request, Luca Fascione, 2023/01/20
- Re: updating merge request, Jean Abou Samra, 2023/01/20
Re: updating merge request,
Lukas-Fabian Moser <=