Merging Of Contributions

Hi Andreas,

Hi Paolo,

Hi Andreas,

I wonder if it is useful to open a second (or more than that)
communication channel for this with Gerrit for code review (with an
email messaging) in place. I thought that Gerrit has been set up to make
the communication about patches more easy and help (volunteer)
developers. But maybe that's not intended with that project resource.

I'm not a developer so I probably cannot help directly but I'm
interested to know what issues you are experiencing.

I kind of understand you have submitted a patch to fix an issue and
you are waiting for it to be merged.

yes.

Is the waiting caused by the patch itself that has something wrong,
the tooling or simply someone has been busy and it hasn't been
reviewed yet?

The patch was reviewed by Jenkins and marked by a developer with +2 and
is marked ready to be merged.

OK so if it's ready to be merged what is stopping it from happening?

Sorry for probably asking silly questions but having more details
might help me and others understand how to avoid that issue.

I think there are no silly questions.  Hope my answer helps to better
understand the situation.

Sorry but it kind of made it more difficult to understand for me.

You seem to say that it's all fine and ready to go but then something is still not right.

Is there an issue with merging patches in general or with merging patches related to specific components?

Can you share a link to it so that maybe someone looks at it and spots the issue?

Regards,
Andreas

Ciao

Paolo

Hi Paolo, all,

Hi Andreas,

Hi Paolo,

Hi Andreas,

I wonder if it is useful to open a second (or more than that)
communication channel for this with Gerrit for code review (with an
email messaging) in place. I thought that Gerrit has been set up to
make
the communication about patches more easy and help (volunteer)
developers. But maybe that's not intended with that project resource.

I'm not a developer so I probably cannot help directly but I'm
interested to know what issues you are experiencing.

I kind of understand you have submitted a patch to fix an issue and
you are waiting for it to be merged.

yes.

Is the waiting caused by the patch itself that has something wrong,
the tooling or simply someone has been busy and it hasn't been
reviewed yet?

The patch was reviewed by Jenkins and marked by a developer with +2 and
is marked ready to be merged.

OK so if it's ready to be merged what is stopping it from happening?

I don't know. There is no message on Gerrit or somewhere else about an
issue which hinders merging.

Sorry for probably asking silly questions but having more details
might help me and others understand how to avoid that issue.

I think there are no silly questions.  Hope my answer helps to better
understand the situation.

Sorry but it kind of made it more difficult to understand for me.

You seem to say that it's all fine and ready to go but then something
is still not right.

Is there an issue with merging patches in general or with merging
patches related to specific components?

Can you share a link to it so that maybe someone looks at it and spots
the issue?

It is this patch (only a small one with two lines of code):

https://gerrit.libreoffice.org/c/core/+/138884

Regards,
Andreas

Hi all,

I think it is about this patch:
https://gerrit.libreoffice.org/c/core/+/138884
It has a +2 from Adolfo Jayme Barrientos but is not yet submitted.

Kind regards,
Regina

Given TDF separates administrative and technical responsibilities between the Board and ESC respectively, I am unclear why this matter is being raised here - has it already been raised at ESC without a resolution? Can someone explain please?

Cheers

Simon

On Thu, Sep 1, 2022 at 3:37 PM Andreas Mantke <maand@gmx.de> wrote:

Hi Thorsten, hi all,

Am 31.08.22 um 21:28 schrieb Thorsten Behrens:

Hi Andreas,

Andreas Mantke wrote:

I wonder what is the usual time slot it needs to merge a reviewed
and ready patch into the LibreOffice repository.

Answering as a developer here (and not as board member) - I think you
will get better answers & suggestions what to do, on the LibreOffice
developer list, or on IRC (see [1] for details).

Please also include a link to your concrete submission, so people can
jump right in.

[1] https://wiki.documentfoundation.org/Development/GetInvolved#Connect_to_our_communication_channels

I wonder if it is useful to open a second (or more than that)
communication channel for this with Gerrit for code review (with an
email messaging) in place. I thought that Gerrit has been set up to make
the communication about patches more easy and help (volunteer)
developers. But maybe that’s not intended with that project resource.

Regards,
Andreas

Free Software Advocate

Plone add-on developer

My blog: http://www.amantke.de/blog


To unsubscribe e-mail to: board-discuss+unsubscribe@documentfoundation.org
Problems? https://www.libreoffice.org/get-help/mailing-lists/how-to-unsubscribe/
Posting guidelines + more: https://wiki.documentfoundation.org/Netiquette
List archive: https://listarchives.documentfoundation.org/www/board-discuss/
Privacy Policy: https://www.documentfoundation.org/privacy

Hi Guilhem,

I wonder what is the usual time slot it needs to merge a reviewed and
ready patch into the LibreOffice repository.

https://dashboard.documentfoundation.org/app/kibana#/dashboard/Gerrit-Timing and
https://dashboard.documentfoundation.org/app/kibana#/dashboard/Gerrit-Backlog
have average and median timing metrics per repo and developer, and one
can tweak the filter to add the desired conditions. For more complex queries
you can use the elasticsearch API, but if there is interest in adding more
visuals to the dashboard we can also do that of course.

thanks for your explanation and the links.

Regards,
Andreas

Hi Andreas,

On 01/09/2022 17:36, Andreas Mantke wrote:

It is this patch (only a small one with two lines of code):

https://gerrit.libreoffice.org/c/core/+/138884

I see a comment “The correct naming for LibreOffice is lool”

and a request to change a variable ending with cool into lool in LibreOffice core.

I remember variable changing going on when LOOL has been forked into COOL, and the removal of "This file is part of the LibreOffice project :-(, but I thought that it was affecting only their own repository, not LibreOffice core so I’m surprised we have references within LO code to a product that isn’t hosted at TDF.

Not sure if any other project can/should do that and if that affects others eg. does that variable change create issues to those that are still using LOOL or something similar?

Does that create issues for you?

Regards,
Andreas

Ciao

Paolo

Hi Simon,

I think, a large delay between review and submit is not a technical problem, but it needs a person who is responsible for an overview.
A technical problem is to provide a tool for a quick overview. However, the dashboard-links from Guilhem look promising.

A large delay is frustrating for the volunteer contributor and is contrary to a welcome culture.

Kind regards
Regin

Hi Paolo, all,

Hi Andreas,

It is this patch (only a small one with two lines of code):

https://gerrit.libreoffice.org/c/core/+/138884

I see a comment "The correct naming for LibreOffice is lool"

and a request to change a variable ending with cool into lool in
LibreOffice core.

I remember variable changing going on when LOOL has been forked into
COOL, and the removal of "This file is part of the LibreOffice project
:-(, but I thought that it was affecting only their own repository,
not LibreOffice core so I'm surprised we have references within LO
code to a product that isn't hosted at TDF.

I thought the same, but ... This was the first such thing I stumbled upon.

Not sure if any other project can/should do that and if that affects
others eg. does that variable change create issues to those that are
still using LOOL or something similar?

Does that create issues for you?

It creates issues on slideshow touch gestures in the mobile app. The
Android and the IoS apps are also part of the online repository.

Regards,
Andreas

Hi Regina,

It’s still the ESC’s role to resolve it, not the Board’s. All the work on code is done by individuals, and open source projects don’t get to instruct individuals on what to do or when to do it - there is no SLA. If none of those individuals are interested in prioritising any particular patch, the correct next step is to raise it at the ESC, which exists expressly to resolve differences between the individuals working on the code. TDF’s Board is not expected to intervene in the ESC’s work beyond its recent approval of ESC appointees. The request here is out-of-order.

While delays are frustrating - my own bug reports remain unfixed after a decade - it is also very frustrating to see a contributor circumvent TDF’s well-tested processes, especially for political ends.

Cheers

Simon

Hi Andreas,

On 01/09/2022 18:48, Andreas Mantke wrote:

I remember variable changing going on when LOOL has been forked into
COOL, and the removal of "This file is part of the LibreOffice project
:-(, but I thought that it was affecting only their own repository,
not LibreOffice core so I’m surprised we have references within LO
code to a product that isn’t hosted at TDF.

I thought the same, but … This was the first such thing I stumbled upon.

I guess we’ll have to understand if that is something that should be monitored or not.

Changes like that might affect others that are working on other platforms linked to LibreOffice core and to me it’s not clear if there is or there should be a policy for that.

Not sure if any other project can/should do that and if that affects
others eg. does that variable change create issues to those that are
still using LOOL or something similar?

Does that create issues for you?

It creates issues on slideshow touch gestures in the mobile app. The
Android and the IoS apps are also part of the online repository.

So that is a change in LibreOffice core that breaks your attempt of creating a LOOL derived implementation?

Wondering if it affects also others like OSSII or similar similar projects based on LibreOffice Technology.

Regards,
Andreas

Ciao

Paolo

Hi Paolo,

Hi Andreas,

I remember variable changing going on when LOOL has been forked into
COOL, and the removal of "This file is part of the LibreOffice project
:-(, but I thought that it was affecting only their own repository,
not LibreOffice core so I'm surprised we have references within LO
code to a product that isn't hosted at TDF.

I thought the same, but ... This was the first such thing I stumbled
upon.

I guess we'll have to understand if that is something that should be
monitored or not.

Changes like that might affect others that are working on other
platforms linked to LibreOffice core and to me it's not clear if there
is or there should be a policy for that.

I think there should be clear rules that there should not be any
barriers or something similar in the LibreOffice vanilla code.

But once I saw the changes from 'lool' to 'cool' in the source code I
wonder why the ESC didn't discussed and stopped it.

Not sure if any other project can/should do that and if that affects
others eg. does that variable change create issues to those that are
still using LOOL or something similar?

Does that create issues for you?

It creates issues on slideshow touch gestures in the mobile app. The
Android and the IoS apps are also part of the online repository.

So that is a change in LibreOffice core that breaks your attempt of
creating a LOOL derived implementation?

Wondering if it affects also others like OSSII or similar similar
projects based on LibreOffice Technology.

It will, if they follow the naming convention of LibreOffice and didn't
rename everything to 'cool' or including 'cool' instead of 'lool'.

Regards,
Andreas

Hi all,

TDF's Board is not expected to intervene in the ESC's work beyond its recent approval of ESC appointees. The request here is out-of-order.

I was wondering as well why Andreas would send a request to board-discuss for a patch.

While delays are frustrating - my own bug reports remain unfixed after a decade - it is also very frustrating to see a contributor circumvent TDF's well-tested processes, especially for political ends.

[Communication issue: I'm very surprised to see that you are accusing someone of having "political ends" as if it was the most normal thing to say.
This is not just a "passive aggressive insinuation", it's an actual direct accusation and if you have evidence of mal-intent you should present it together with the accusation. Keep in mind that some got a CoC complaint for a lot less than that (sorry I can't disclose names as complaints should be kept private).]

Looking at the request, then at the patch and the latest description provided I understood that Andreas is working on a project that has a very short window of opportunity set by the Board itself and he's doing his best to get some results as fast as possible. So in a way I kind of understand the attempt as the board itself was involved in setting limits that make what is doing very time sensitive even if it's hardly a justifiable excuse to bypass standard procedures.

While I agree that the ESC should be involved in code related matters this seems to go beyond that as variables in LibreOffice core have been changed to represent the initials of a project not hosted at TDF (LOOL -> COOL) and if I've understood well the change breaks other projects that use the initials of the project still hosted at TDF (LOOL).

That, IMHO, isn't just matter of "code" as a change that introduces a commercial product name in LibreOffice core should have been cleared with the board which I believe would have not agree to it as LibreOffice should be "brand neutral" and create a level playing field for all contributors being voluntary or commercial.
Not being a developer I might get things mixed up but I guess the board should evaluate if the necessary policies and checks are in place.

Ciao

Paolo

Hi Andreas,

I think there should be clear rules that there should not be any
barriers or something similar in the LibreOffice vanilla code.

But once I saw the changes from 'lool' to 'cool' in the source code I
wonder why the ESC didn't discussed and stopped it.

I've noticed your suggestion after I sent my other answer.

I'm surprised that the ESC didn't contact the board to notify us that some changes were more like a "branding" changes that actual improvements necessary in LibreOffice core.

I don't know if the ESC could have done anything about it, I guess it's hard to spot a cool among many lool but then we'll have to notify all developers that branding, which on top of it break things for other projects, in core is not or should not be allowed.

Ciao

Paolo

What a silly thread.

  Some neutral name for the integration should be picked and
implemented and/or both names be accepted; I would suggest 'lok'
for LibreOfficeKit. Commit/revert wars are a nonsense that helps
no-one - I would expect the ESC should intervene and cut out the
politics.

  There are of course three big problems in computer science:
naming things, and off-by-one errors - so it's no real surprise.

I guess it's hard to spot a cool among many lool but then
we'll have to notify all developers...

  LibreOffice is much cooler than you think; cf. the appended.

    Michael.

$ git grep lool
i18npool/source/localedata/data/om_ET.xml: <DefaultFullName>Onkoloolessa </DefaultFullName>

$ git grep cool
canvas/workben/canvasdemo.cxx: //TODO: do something cool
extras/source/autocorr/lang/en-AU/DocumentList.xml: <block-list:block block-list:abbreviated-name="shcool" block-list:name="school"/>
extras/source/autocorr/lang/en-GB/DocumentList.xml: <block-list:block block-list:abbreviated-name="shcool" block-list:name="school"/>
extras/source/autocorr/lang/en-US/DocumentList.xml: <block-list:block block-list:abbreviated-name="shcool" block-list:name="school"/>
extras/source/autocorr/lang/en-ZA/DocumentList.xml: <block-list:block block-list:abbreviated-name="shcool" block-list:name="school"/>
extras/source/autocorr/lang/ja/DocumentList.xml: <block-list:block block-list:abbreviated-name="shcool" block-list:name="school"/>
extras/source/autocorr/lang/ko/DocumentList.xml: <block-list:block block-list:abbreviated-name="shcool" block-list:name="school"/>
extras/source/autocorr/lang/zh-CN/DocumentList.xml: <block-list:block block-list:abbreviated-name="shcool" block-list:name="school"/>
extras/source/autocorr/lang/zh-TW/DocumentList.xml: <block-list:block block-list:abbreviated-name="shcool" block-list:name="school"/>
filter/source/svg/presentation_engine.js: window.webkit.messageHandlers.cool !== undefined)
filter/source/svg/presentation_engine.js: window.webkit.messageHandlers.cool.postMessage('EXITSLIDESHOW', '*');
oox/source/drawingml/shape3dproperties.cxx: case XML_coolSlant: return "coolSlant";
oox/source/token/tokens.txt:coolSlant
readlicense_oo/license/CREDITS.fodt: <text:p text:style-name="Table_20_Contents"><text:a xlink:type="simple" xlink:href="http://wiki.documentfoundation.org/User:Maahicool" text:style-name="Internet_20_link" text:visited-style-name="Visited_20_Internet_20_Link">Maahicool</text:a> (1) </text:p>
reportbuilder/java/org/libreoffice/report/pentaho/output/ImageProducer.java: // cool, the file exists. Let's try to read it.
sd/source/filter/html/htmlex.cxx: // Exceptions are cool...
sfx2/emojiconfig/emoji.json: "cool": {
sfx2/emojiconfig/emoji.json: "name": "squared cool",
sfx2/emojiconfig/emoji.json: "shortname": ":cool:",
solenv/inc/mime.types:x-conference/x-cooltalk ice
sw/source/core/doc/tblrwcl.cxx: // It *is* sometimes cool to have multiple tests/if's and assignments
sw/source/filter/ww8/ww8scan.cxx: Otherwise our cool fastsave algorithm can be brought to bear on the
sw/source/filter/ww8/ww8scan.cxx: //Store old end position for supercool new property finder that uses
vcl/unx/generic/print/common_gfx.cxx: // cool, we can concatenate rectangles
writerfilter/source/dmapper/TextEffectsHandler.cxx: case NS_ooxml::LN_ST_BevelPresetType_coolSlant: return "coolSlant";
writerfilter/source/ooxml/model.xml: <value>coolSlant</value>
writerfilter/source/ooxml/model.xml: <value tokenid="ooxml:Value_drawingml_ST_BevelPresetType_coolSlant">coolSlant</value>
writerfilter/source/ooxml/model.xml: <value>coolSlant</value>
writerfilter/source/ooxml/model.xml: <value tokenid="ooxml:ST_BevelPresetType_coolSlant">coolSlant</value>

Hi all,

I would expect the ESC should intervene and cut out the
politics.

I believe that politics have nothing to do with what happened.

Your grep shows that there aren't that many "lool" that could be renamed "cool" by mistake so I guess it's unlikely for it to happen again.

I've noticed the very positive and prompt review of this incident by Thorsten so it's nice to see that solutions that work for all can be found quickly when the will is there :wink:

Ciao

Paolo

Hi all,

Hi all,

I would expect the ESC should intervene and cut out the
politics.

I believe that politics have nothing to do with what happened.

Your grep shows that there aren't that many "lool" that could be
renamed "cool" by mistake so I guess it's unlikely for it to happen
again.

I've noticed the very positive and prompt review of this incident by
Thorsten so it's nice to see that solutions that work for all can be
found quickly when the will is there :wink:

I try a bit to give an overview on the latest changes of the file in
discussion:

The naming in the source code contains 'lool' in it until it was changed
five month ago to 'cool'. Thus the original naming was:

window.webkit.messageHandlers.lool !== undefined)
window.webkit.messageHandlers.lool.postMessage('EXITSLIDESHOW', '*');

The change from 'lool' to 'cool' created a break for at least two known
downstream consumer projects.

In my view it is not usual for an OSS project to support within its main
branch downstream consumer projects which make a breaking change in
their own external hosted code.

If we'd do that the next downstream consumer project probably asks
LibreOffice and its voluteers to support a further (naming) break (e.g.
naming it 'silly').

Thus the first solution would be to get the old naming back ('lool') and
fix the break for at least two downstream consumer projects. This was
done with the submitted patch.

Then there were a complain that this would break one downstream consumer
project and I as a volunteer should provide a fix also for that project.
There are two reasons why I think this couldn't be a solution. It's not
a task for a volunteer to fix issues introduced within the external
hosted source code of that project. And if TDF and its community would
do the fix this way another consumer project with a breaking change
could ask for the same.

But if you grep to the LibreOffice source code within core you'll find
out that there are outside this file no other namings of variables ,
functions etc. with 'lool' or 'cool' inside. Thus the naming in the file
could be viewed as used by fault. I share Michael's view in this case. I
grepped also for 'lok' with a much bigger output. Thus as Michael and
also Miklos proposed the renaming with 'lok' instead of 'cool' could be
a solution. But in the end this would break not only one or two
downstream consumer projects but all three known.

If I look at the Python world there are always deprecation warnings long
time before a breaking change. I'd like to avoid the need of such a
time-frame. We currently know that at least three projects are affected
from the naming change. Thus I propose to provide in addition a code
patch and make a pull request to every of this three downstream consumer
projects.

Regards,
Andreas

Hi,

Hi all,

Hi all,

I would expect the ESC should intervene and cut out the
politics.

I believe that politics have nothing to do with what happened.

Your grep shows that there aren’t that many “lool” that could be
renamed “cool” by mistake so I guess it’s unlikely for it to happen
again.

I’ve noticed the very positive and prompt review of this incident by
Thorsten so it’s nice to see that solutions that work for all can be
found quickly when the will is there :wink:

I try a bit to give an overview on the latest changes of the file in
discussion:

The naming in the source code contains ‘lool’ in it until it was changed
five month ago to ‘cool’. Thus the original naming was:

window.webkit.messageHandlers.lool !== undefined)
window.webkit.messageHandlers.lool.postMessage(‘EXITSLIDESHOW’, ‘*’);

The change from ‘lool’ to ‘cool’ created a break for at least two known
downstream consumer projects.

Just for the record, the incriminated code is used solely by the iOS app (Collabora Office). Are there other iOS apps that broke because of the lool->cool change? As far as I’m concerned there aren’t any. It’s good that we discussed the issue at length, and will have a vendor neutral message handler name finally. :wink:

Regards,
Andras

Hi,

Hi,

    Hi all,

    > Hi all,
    >
    >> I would expect the ESC should intervene and cut out the
    >> politics.
    >
    > I believe that politics have nothing to do with what happened.
    >
    > Your grep shows that there aren't that many "lool" that could be
    > renamed "cool" by mistake so I guess it's unlikely for it to happen
    > again.
    >
    > I've noticed the very positive and prompt review of this incident by
    > Thorsten so it's nice to see that solutions that work for all can be
    > found quickly when the will is there :wink:
    >
    I try a bit to give an overview on the latest changes of the file in
    discussion:

    The naming in the source code contains 'lool' in it until it was
    changed
    five month ago to 'cool'. Thus the original naming was:

    window.webkit.messageHandlers.lool !== undefined)
    window.webkit.messageHandlers.lool.postMessage('EXITSLIDESHOW', '*');

    The change from 'lool' to 'cool' created a break for at least two
    known
    downstream consumer projects.

Just for the record, the incriminated code is used solely by the iOS
app (Collabora Office). Are there other iOS apps that broke because of
the lool->cool change? As far as I'm concerned there aren't any. It's
good that we discussed the issue at length, and will have a vendor
neutral message handler name finally. :wink:

I'd appreciate if you read my message completely. And I'd be curious if
there is common opinion among you and your colleagues. Currently I'm a
bit puzzled.

Regards,
Andreas

Hi Andras,

On 02/09/2022 18:47, Andras Timar wrote:

Hi,

On Fri, Sep 2, 2022 at 5:38 PM Andreas Mantke <maand@gmx.de> wrote:

Hi all,

Am 01.09.22 um 23:16 schrieb Paolo Vecchi:

Hi all,

On 01/09/2022 22:47, Michael Meeks wrote:

I would expect the ESC should intervene and cut out the
politics.

I believe that politics have nothing to do with what happened.

Your grep shows that there aren’t that many “lool” that could be
renamed “cool” by mistake so I guess it’s unlikely for it to happen
again.

I’ve noticed the very positive and prompt review of this incident by
Thorsten so it’s nice to see that solutions that work for all can be
found quickly when the will is there :wink:

I try a bit to give an overview on the latest changes of the file in
discussion:

The naming in the source code contains ‘lool’ in it until it was changed
five month ago to ‘cool’. Thus the original naming was:

window.webkit.messageHandlers.lool !== undefined)
window.webkit.messageHandlers.lool.postMessage(‘EXITSLIDESHOW’, ‘*’);

The change from ‘lool’ to ‘cool’ created a break for at least two known
downstream consumer projects.

Just for the record, the incriminated code is used solely by the iOS app (Collabora Office).

Apparently not?

Are there other iOS apps that broke because of the lool->cool change?

Isn’t Andreas saying that it broke two downstream consumer projects?

As far as I’m concerned there aren’t any.

It seems to transpire that at least 2 other projects were concerned by it.

It’s good that we discussed the issue at length, and will have a vendor neutral message handler name finally. :wink:

It would have been great to have a discussion before the “lool” handler was changed to a “non-vendor neutral” one.

If I understood well a change to “lok” would break even more projects so what about reverting it to a vendor neutral name like “lool” so that the issue is fixed for 3 downstream consumer projects?

Regards,
Andras

Ciao

Paolo

Hi Andreas, all,

Andreas Mantke wrote:

> Just for the record, the incriminated code is used solely by the iOS
> app (Collabora Office). Are there other iOS apps that broke because of
> the lool->cool change? As far as I'm concerned there aren't any. It's
> good that we discussed the issue at length, and will have a vendor
> neutral message handler name finally. :wink:

I'd appreciate if you read my message completely. And I'd be curious if
there is common opinion among you and your colleagues. Currently I'm a
bit puzzled.

Let's please assume good intentions on all sides. TTBOMK, there's
indeed currently only Collabora providing binaries for iOS.

But honestly, that's all besides the point, and this entire thread is
dealing with the issue from the wrong end.

Andreas, can you please first and foremost interact with the reviewers
on gerrit? I'm convinced we can address all remaining questions there
& get a suitable change merged.

And **only** if that fails, it's justified to consume more board &
member's time here. Right now, there's purely technical matters to be
solved.

Cheers,

-- Thorsten