New merge request. Is it correct?

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

New merge request. Is it correct?

Caio Barros
Hello, everyone!

I'be been out for the past couple of weeks due to some issues with work and family, but now, hopefully, I can go back to contribute more regularly.

I just submited a new merge request to the translation branch at https://gitlab.com/lilypond/lilypond/-/merge_requests/141. Can you confirm it is correct? My workflow was the following:

1. fork the lilypond gitlab repository
2. checkout to the translation branch with
git checkout --track origin/translation
3. create a local branch from the translation one
4. push to my forked repository
5. opening a merge request, changing the destination branch from
lilypond/lilypond:master
to 
lilypond/lilypond:translation

I found steps 2 and 5 particularly non-intuitive, so if this worflow is correct, I propose we change the contributor's manual to explain them.  

The explanation of the fix is in the merge request itself. It should remove the text '\input texinfo' which is currently appearing in some pages of the pt-br version of the website (including the homepage)

Caio

+Federico Bruni
+Rafael Fontenelle
Reply | Threaded
Open this post in threaded view
|

Re: New merge request. Is it correct?

Federico Bruni-3


On Mon, 8 Jun, 2020 at 16:16, Caio Barros <[hidden email]> wrote:

> Hello, everyone!
>
> I'be been out for the past couple of weeks due to some issues with
> work and family, but now, hopefully, I can go back to contribute more
> regularly.
>
> I just submited a new merge request to the translation branch at
> https://gitlab.com/lilypond/lilypond/-/merge_requests/141. Can you
> confirm it is correct? My workflow was the following:
>

Hi Caio

I've already replied on GitLab.. but here I will add comments on point
2 and 5.

> 1. fork the lilypond gitlab repository
> 2. checkout to the translation branch with
> git checkout --track origin/translation
> 3. create a local branch from the translation one
> 4. push to my forked repository
> 5. opening a merge request, changing the destination branch from
> lilypond/lilypond:master
> to
> lilypond/lilypond:translation
>
> I found steps 2 and 5 particularly non-intuitive, so if this worflow
> is correct, I propose we change the contributor's manual to explain
> them.
>

2. You should have used simply

  git checkout translation

as you don't have a local branch named translation after a fresh `git
clone` and when you check out the remote translation branch your git
config should be set automatically to track origin/translation.

The CG is too much detailed in my opinion. In order not to duplicate
information (good), you are forced to follow several links (bad) and
read a git tutorial when all you have to do is typing two simple
commands.  A translator starts from here:

http://lilypond.org/doc/v2.21/Documentation/contributor/getting-started-with-documentation-translation

then he/she will follow the links but will never find the two simple
commands to get started (git pull REMOTE && git checkout translation).
I'd rather put the commands in that page and maybe keep the link to
"Starting with Git" just for further information.

5. This is a valid point, but I think that Jonas (here in Cc) did not
take translation into account, as most translators have write access
and push their work without asking for a review (I think that active
languages have only one maintainer).
Don't know if GitLab may be able to understand that a MR is created
from the translation branch and therefore change the target branch to
translation...


> The explanation of the fix is in the merge request itself. It should
> remove the text '\input texinfo' which is currently appearing in some
> pages of the pt-br version of the website (including the homepage)
>

>
Now I found it :-)
http://lilypond.org/doc/v2.21/Documentation/web/index.pt.html






Reply | Threaded
Open this post in threaded view
|

Re: New merge request. Is it correct?

Caio Barros


Em qua., 10 de jun. de 2020 às 06:13, Jonas Hahnfeld <[hidden email]> escreveu:
Am Dienstag, den 09.06.2020, 23:51 +0200 schrieb Federico Bruni:
> On Mon, 8 Jun, 2020 at 16:16, Caio Barros <[hidden email]> wrote:
> > [...]
> > 5. opening a merge request, changing the destination branch from
> > lilypond/lilypond:master
> > to
> > lilypond/lilypond:translation
> >
> > I found steps 2 and 5 particularly non-intuitive, so if this worflow
> > is correct, I propose we change the contributor's manual to explain
> > them.
>
> [...]
>
> 5. This is a valid point, but I think that Jonas (here in Cc) did not
> take translation into account, as most translators have write access
> and push their work without asking for a review (I think that active
> languages have only one maintainer).

Yes, I've seen only few translation patches go through the usual review
process since I started working on LilyPond last year. IIRC all of them
targeted master and were not pushed to translation first.

In that case, do you think it is useful to change the documentation to add these steps to commit to the translation branch? From what I understand, this would mean to change the workflow of virtually all translators except (maybe) new ones. Perhaps we can just carry on pushing to master and add a label to translation MRs.

Actually, because the project moved to GitLab a lot of sections in the CG are outdated. Is there anyone in charge of updating the CG? Since I'm fairly new in contributing I've been noticing some things that got me confused there, maybe I can suggest some modifications.
 
> Don't know if GitLab may be able to understand that a MR is created
> from the translation branch and therefore change the target branch to
> translation...

As far as I know, GitLab will automatically target the default branch.
This is 'master', and that choice is probably correct for 99% of the
cases.

Yes, when I open the merge request URL it automatically sets the merge request to master. I have chose to change MR !141 to the translation branch because of the section on CG on git [1].
 
That said, if we want to use merge request for translations in the
future, we can surely figure out improvements for that workflow. Most
importantly, the merge request !141 cannot currently be merged because
it is missing automated testing. I think having some dummy check
suffices here, the "real" testing takes place when merging translation
to master.

Well, the merge was approved by Werner Lemberg. Should I send a new merge request to master? I don't understand.

Caio