PATCHES - Countdown for May 15th

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

PATCHES - Countdown for May 15th

James Lowe-9
Hello,

Here is the current patch countdown list. The next countdown will be on
May 17th.

A list of all merge requests can be found here:
https://gitlab.com/lilypond/lilypond/-/merge_requests?sort=label_priority


      Push:

!21 Update CG to state Python 3 requirement - Jean Abou Samra
https://gitlab.com/lilypond/lilypond/-/merge_requests/21

!19 ignore settings for the vscode editor - Jaap de Wolff
https://gitlab.com/lilypond/lilypond/-/merge_requests/19

!17 Use vsize for source files - Jaap de Wolff
https://gitlab.com/lilypond/lilypond/-/merge_requests/17

!16 Quote_iterator: use vsize for index - Jaap de Wolff
https://gitlab.com/lilypond/lilypond/-/merge_requests/16

!15 ignore conversion warning in lexer.cc - Jaap de Wolff
https://gitlab.com/lilypond/lilypond/-/merge_requests/15

!14 do not use depcrecated %error-verbose (bison 2013) - Jaap de Wolff
https://gitlab.com/lilypond/lilypond/-/merge_requests/14

!13 Modify obvious incorrect types - Jaap de Wolff
https://gitlab.com/lilypond/lilypond/-/merge_requests/13

!12 Valid declaration of unused arguments - Jaap de Wolff
https://gitlab.com/lilypond/lilypond/-/merge_requests/12

!11 Sequential_iterator maintenance - Dan Eble
https://gitlab.com/lilypond/lilypond/-/merge_requests/11

!9 Issue 4182: avoid checking the offset of cross-staff stems too early
- Kevin Barry
https://gitlab.com/lilypond/lilypond/-/merge_requests/9

!8 Resolve "Remove deprecated context properties" - Valentin Villenave
https://gitlab.com/lilypond/lilypond/-/merge_requests/8

!7 Resolve "Multi_measure_rest_engraver segfaults when its context
doesn’t include Staff_symbol_engraver." - Valentin Villenave
https://gitlab.com/lilypond/lilypond/-/merge_requests/7

!5 Enable 'relative-includes by default. - Valentin Villenave
https://gitlab.com/lilypond/lilypond/-/merge_requests/5


      Countdown:

!37 Fix #5234: Segfault when music is empty and only MIDI file is
generated - Jaap de Wolff
https://gitlab.com/lilypond/lilypond/-/merge_requests/37

!32 Percent_repeat_iterator: stop gratuitous use of std::string - David
Kastrup
https://gitlab.com/lilypond/lilypond/-/merge_requests/32

!31 Lilymidy: open midi files in binary mode - Martin Neubauer
https://gitlab.com/lilypond/lilypond/-/merge_requests/31

!30 Remove break-visibility handling in tablature ties - Han-Wen Nienhuys
https://gitlab.com/lilypond/lilypond/-/merge_requests/30

!29 Comment fixes for offset_directed - David Kastrup
https://gitlab.com/lilypond/lilypond/-/merge_requests/29

!28 Resolve "Use a callback for Percent_repeat_iterator's repeat action"
- David Kastrup
https://gitlab.com/lilypond/lilypond/-/merge_requests/28

!27 Use `-dfont-ps-resdir` for make doc - Masamichi Hosoda
https://gitlab.com/lilypond/lilypond/-/merge_requests/27

!26 Split glyph contours in up/down segments for skylines - Han-Wen Nienhuys
https://gitlab.com/lilypond/lilypond/-/merge_requests/26

!24 Cache the name => index lookup in Open_type_font - Han-Wen Nienhuys
https://gitlab.com/lilypond/lilypond/-/merge_requests/24

!20 Add regtest for multiple post-events wrapper behavior - Valentin
Villenave
https://gitlab.com/lilypond/lilypond/-/merge_requests/20

!3 Add dynamic-interface to keepAliveInterfaces - Jean Abou Samra
https://gitlab.com/lilypond/lilypond/-/merge_requests/3


      Review:

!34 Normal output to stdout - Jaap de Wolff
https://gitlab.com/lilypond/lilypond/-/merge_requests/34

!22 Doc: remove now unnecessary dummy argument after lilypond
-dshow-available-fonts - Jean Abou Samra
https://gitlab.com/lilypond/lilypond/-/merge_requests/22

!4 Ensure having valid references to the position in the source file
while... - Martin Neubauer
https://gitlab.com/lilypond/lilypond/-/merge_requests/4


      New:

No patches in New at this time.


      Waiting:

No patches in Waiting at this time.





*******

Regards,

James

Reply | Threaded
Open this post in threaded view
|

Re: PATCHES - Countdown for May 15th

Jonas Hahnfeld
Am Freitag, den 15.05.2020, 13:44 +0100 schrieb James:

> Hello,
>
> Here is the current patch countdown list. The next countdown will be on
> May 17th.
>
> A list of all merge requests can be found here:
> https://gitlab.com/lilypond/lilypond/-/merge_requests?sort=label_priority
>
>
>
>       Push:
To push patches and automatically close the merge request, GitLab needs
to know about the rebased commits. This is easiest if you click
"Rebase" in the UI, but doing so locally and force-pushing the branch
is also fine. Afterwards you can either click "Merge" (I verified that
all target staging instead of master) or just push the identical set of
commits to staging.

The second part only works if you are a member of the group on GitLab.
If not, you need somebody from the team to merge for you. Still the
commits need to be rebased (and possibly squashed). This is easiest if
you give us permission to do so, which is documented here:
https://docs.gitlab.com/ee/user/project/merge_requests/allow_collaboration.html
Otherwise we'll probably need to go through multiple iterations of you
rebasing and us trying to merge until it is fast-forward...

Regards
Jonas

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PATCHES - Countdown for May 15th

James Lowe-9

On 15/05/2020 14:16, Jonas Hahnfeld wrote:

> Am Freitag, den 15.05.2020, 13:44 +0100 schrieb James:
>> Hello,
>>
>> Here is the current patch countdown list. The next countdown will be on
>> May 17th.
>>
>> A list of all merge requests can be found here:
>> https://gitlab.com/lilypond/lilypond/-/merge_requests?sort=label_priority
>>
>>
>>
>>        Push:
> To push patches and automatically close the merge request, GitLab needs
> to know about the rebased commits. This is easiest if you click
> "Rebase" in the UI, but doing so locally and force-pushing the branch
> is also fine. Afterwards you can either click "Merge" (I verified that
> all target staging instead of master) or just push the identical set of
> commits to staging.
>
> The second part only works if you are a member of the group on GitLab.
> If not, you need somebody from the team to merge for you. Still the
> commits need to be rebased (and possibly squashed). This is easiest if
> you give us permission to do so, which is documented here:
> https://docs.gitlab.com/ee/user/project/merge_requests/allow_collaboration.html
> Otherwise we'll probably need to go through multiple iterations of you
> rebasing and us trying to merge until it is fast-forward...
>
> Regards
> Jonas

So what Jaap is doing and triggering re-instatement of Patch::label is a
mistake?

I've just got a lot of 'push' patches back to 'new'.

James


Reply | Threaded
Open this post in threaded view
|

RE: PATCHES - Countdown for May 15th

lilypond-5
I did a rebase, that triggered it

> -----Oorspronkelijk bericht-----
> Van: lilypond-devel <lilypond-devel-bounces+lilypond=de-
> [hidden email]> Namens James
> Verzonden: Friday, May 15, 2020 3:53 PM
> Aan: Jonas Hahnfeld <[hidden email]>; lilypond-devel <lilypond-
> [hidden email]>
> Onderwerp: Re: PATCHES - Countdown for May 15th
>
>
> On 15/05/2020 14:16, Jonas Hahnfeld wrote:
> > Am Freitag, den 15.05.2020, 13:44 +0100 schrieb James:
> >> Hello,
> >>
> >> Here is the current patch countdown list. The next countdown will be
> >> on May 17th.
> >>
> >> A list of all merge requests can be found here:
> >> https://gitlab.com/lilypond/lilypond/-/merge_requests?sort=label_prio
> >> rity
> >>
> >>
> >>
> >>        Push:
> > To push patches and automatically close the merge request, GitLab
> > needs to know about the rebased commits. This is easiest if you click
> > "Rebase" in the UI, but doing so locally and force-pushing the branch
> > is also fine. Afterwards you can either click "Merge" (I verified that
> > all target staging instead of master) or just push the identical set
> > of commits to staging.
> >
> > The second part only works if you are a member of the group on GitLab.
> > If not, you need somebody from the team to merge for you. Still the
> > commits need to be rebased (and possibly squashed). This is easiest if
> > you give us permission to do so, which is documented here:
> > https://docs.gitlab.com/ee/user/project/merge_requests/allow_collabora
> > tion.html Otherwise we'll probably need to go through multiple
> > iterations of you rebasing and us trying to merge until it is
> > fast-forward...
> >
> > Regards
> > Jonas
>
> So what Jaap is doing and triggering re-instatement of Patch::label is a
> mistake?
>
> I've just got a lot of 'push' patches back to 'new'.
>
> James
>



Reply | Threaded
Open this post in threaded view
|

Re: PATCHES - Countdown for May 15th

James Lowe-9
Hello,

On 15/05/2020 15:05, [hidden email] wrote:
> I did a rebase, that triggered it

OK Jonas, that might be the first 'wrinkle' for me in the process. To
save me blindly re-testing a 'new patch' labelled issue.

I cannot really tell easily a real new patch and one that is rebased
ready for pushing.

What do you think?

James


Reply | Threaded
Open this post in threaded view
|

Re: PATCHES - Countdown for May 15th

Jonas Hahnfeld
Am Freitag, den 15.05.2020, 15:20 +0100 schrieb 'James':

> Hello,
>
> On 15/05/2020 15:05, [hidden email] wrote:
> > I did a rebase, that triggered it
>
> OK Jonas, that might be the first 'wrinkle' for me in the process. To
> save me blindly re-testing a 'new patch' labelled issue.
>
> I cannot really tell easily a real new patch and one that is rebased
> ready for pushing.
>
> What do you think?
That the script is doing exactly what I told it to do: The diff between
the previous and the rebased commit is not empty. Therefore it adds the
Patch::new label, removing Patch::push.
While correct in theory (new diff = new testing), I think we should
ignore updates once in Patch::push. If the author changes something
which was not in previous versions of the patch, we need to reset to
Patch::new manually. Does this sound acceptable?

Jonas

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PATCHES - Countdown for May 15th

Kevin Barry
On Fri, May 15, 2020 at 04:25:52PM +0200, Jonas Hahnfeld wrote:
> That the script is doing exactly what I told it to do: The diff between
> the previous and the rebased commit is not empty. Therefore it adds the
> Patch::new label, removing Patch::push.

Shouldn't `diff staging...HEAD` be the same before and after a rebase
(three dots)?

Reply | Threaded
Open this post in threaded view
|

Re: PATCHES - Countdown for May 15th

Jonas Hahnfeld
Am Freitag, den 15.05.2020, 16:57 +0100 schrieb Kevin Barry:
> On Fri, May 15, 2020 at 04:25:52PM +0200, Jonas Hahnfeld wrote:
> > That the script is doing exactly what I told it to do: The diff between
> > the previous and the rebased commit is not empty. Therefore it adds the
> > Patch::new label, removing Patch::push.
>
> Shouldn't `diff staging...HEAD` be the same before and after a rebase
> (three dots)?

Yes, I think this works as long as staging not already includes some
(rebased) commits that were previously part of the merge request. We
could of course determine the merge base between the old and new commit
which should also avoid that case.
However this means three API calls (merge base + 2x diff) instead of
one (plain diff), and the webhook is supposed to finish in less than 10
seconds. I'll need to test this...

Jonas

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PATCHES - Countdown for May 15th

Jonas Hahnfeld
Am Freitag, den 15.05.2020, 21:32 +0200 schrieb Jonas Hahnfeld:

> Am Freitag, den 15.05.2020, 16:57 +0100 schrieb Kevin Barry:
> > On Fri, May 15, 2020 at 04:25:52PM +0200, Jonas Hahnfeld wrote:
> > > That the script is doing exactly what I told it to do: The diff between
> > > the previous and the rebased commit is not empty. Therefore it adds the
> > > Patch::new label, removing Patch::push.
> >
> > Shouldn't `diff staging...HEAD` be the same before and after a rebase
> > (three dots)?
>
> Yes, I think this works as long as staging not already includes some
> (rebased) commits that were previously part of the merge request. We
> could of course determine the merge base between the old and new commit
> which should also avoid that case.
My thinking was wrong here: The common merge base of the two commits
results in a larger diff for the new commit because it now includes all
commits since the merge base, which is not what we want.
So I went with Kevin's original suggestion with one modification: The
staging branch is a moving target. Depending on when the webhook runs,
the rebased commits might have already landed which makes the diff
empty. Instead I ask GitLab for the diffs relative to the respective
base commits. I think this does the right thing (tm), at least in my
testing.

I already deployed the change to my server, here for reference:
https://gitlab.com/lilypond/infrastructure/-/merge_requests/4
Testing in the repo and feedback would be welcome!

Jonas

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PATCHES - Countdown for May 15th

James Lowe-9
In reply to this post by Jonas Hahnfeld
On 15/05/2020 15:25, Jonas Hahnfeld wrote:

>> I cannot really tell easily a real new patch and one that is rebased
>> ready for pushing.
>>
>> What do you think?
> That the script is doing exactly what I told it to do: The diff between
> the previous and the rebased commit is not empty. Therefore it adds the
> Patch::new label, removing Patch::push.
> While correct in theory (new diff = new testing), I think we should
> ignore updates once in Patch::push. If the author changes something
> which was not in previous versions of the patch, we need to reset to
> Patch::new manually. Does this sound acceptable?
>
> Jonas

I'll let you and the developers figure that out, I can work with
whatever I need to do. If it means I occasionally retest a patch twice,
then so be it. No big deal.

James