On Thu, Jul 31, 2025 at 7:30 AM Nate Graham <n...@kde.org> wrote:

> On 7/30/25 1:15 PM, Ben Cooksley wrote:
> > If memory serves correctly the general view from that thread could be
> > summarised as:
> > 1) Work branches were basically used to immediately open merge requests
> > in the majority of cases, so running CI on pushes to work branches was
> > simply a waste as CI would be (re-)run on the merge request.
> > 2) Merge requests were used as part of an iterative development
> > workflow, and as such CI runs on merge requests were in many cases also
> > useless because the MR was not yet ready to be merged and was still
> > under active development.
> >
> > Because merge requests that aren't actually ready for final review and
> > then merging cannot currently be distinguished from merge requests that
> > are not at all ready and are just collecting comments, the only way to
> > solve for (2) is to switch CI to manual on merge requests.
> >
> > Gitlab already has a mechanism for differentiating between those two
> > types of merge requests (marking as draft) however it is not used by
> > Plasma (or at least wasn't on the two pages of KWin reviews I quickly
> > skimmed)
> >
> > So this isn't really Gitlab's fault, it is how Plasma has opted to use
> > Gitlab.
>
> Every merge request (everywhere, not just in Plasma) is submitted in a
> state where the author believes it's ready for merging. The purpose of
> review is for others to challenge that belief and offer suggestions for
> improvement. Hence iteration, and avoiding that is impossible for all
> but the smallest and most trivial merge requests.
>

This runs counter to one of the statements in the earlier thread:

[quote]
>Realistically merge requests shouldn't be proposed until people are ready
to get something reviewed and merged in...

That's not realistic with how it is on the frontlines. The earlier we
share feedback the better, and reviews can take many many iterations.
[/quote]

I'm a bit confused here as to what is actually correct?


> You only see this process using a lot of resources in Plasma because
> Plasma is the largest and most active project in KDE. As I recall, there
> were similar complaints about Krita and Kstars, which are also large and
> active projects.
>

Both of those have attracted complaints for other reasons - because they
use custom workflows which then creates a maintenance burden.

Krita in particular has an entirely custom set of CI images, which includes
their own specific compiler toolchain, and has their own set of build
tooling that builds their specific set of dependencies built using CMake
toolchains they maintain.
This includes a patched Qt as well as an entire multimedia toolstack up to
and including ffmpeg as well as a Python build with bindings support.

Stats from the earlier thread:

    time_used     | job_count
------------------+-----------
3536:44:31.03921 |     45860

          full_path           |    time_used     | job_count
------------------------------+------------------+-----------
plasma/kwin                  | 325:40:45.439134 |      2419
plasma/plasma-workspace      | 224:56:33.415042 |      1032
multimedia/kdenlive          | 195:29:20.224873 |       794
graphics/krita               | 189:03:12.118222 |       441
network/ruqola               | 180:30:09.57436  |       585
network/neochat              | 151:25:19.726593 |      1695
education/kstars             | 129:53:37.649402 |       333
sysadmin/ci-management       | 116:32:43.423853 |       163
plasma/plasma-desktop        | 116:07:05.90223  |       818
kde-linux/kde-linux-packages | 84:45:50.156624  |        35


> Implicitly punishing projects for success and popularity is a rather
> perverse incentive, I hope you'll agree!
>

Agreed on that front.

We should strive to run things as efficiently as possible still, which is
something we can do better at.


>
>
>
> > If you were to adopt the draft flag on merge requests that are in that
> > iterative phase and not intended to be merged then we could look at
> > adjusting how it works.
>
> I think we're all mostly in agreement that running the pipeline n times
> for n pushes in large and active merge requests is something that
> doesn't make a lot of sense.
>
> The question is how to improve upon this without throwing the baby out
> with the bathwater.
>

Sounds good to me.


>
> Using the Draft flag for such merge requests once it becomes clear that
> they won't merge immediately is one option. Perhaps there are others,
> like running pipelines automatically only upon MR submission, and
> thereafter requiring a fully green pipeline as a prerequisite for
> merging and starting a pipeline automatically when someone clicks "Set
> to auto-merge". Lots of options here; we can brainstorm.
>

We could set merge request pipelines to always run on an approved merge
request, so the next time it is rebased (which for highly active projects
should basically always be the case by the time approve is ready to be
pressed) the pipeline will kick off?


>
>
>
> > Note that this won't do anything to solve broken tests, or tests that
> > take too long - that is something only Plasma developers can correct.
>
> There's plenty of blame to go around. We can and should improve this.
>

Yes please. I suspect there is a degree of low hanging fruit in the Appium
tests as the first thing they do is install their Python dependencies.
Those should be burned into the image so they're available immediately
without needing to reach out to PyPi.

Given that Appium is practically just an extension of KWin, it should also
be moved within Plasma and follow Plasma versioning practices even if it is
not released alongside Plasma.

This may seem counterintuitive but it is necessary because otherwise Plasma
projects that depend on the Appium tooling (which in turn depends on KWin)
can end up with Plasma dependencies upgraded/replaced when the CI system
completes the build and goes to run tests. These different versions are
sometimes incompatible and have led to breakages in the past. This is due
to each project specifying it's own dependency information - and the Appium
tooling as it does not share Plasma versioning cannot use @same and is
therefore forced to use @latest-kf6 / @stable-kf6. Moving it into Plasma
will allow the use of @same and eliminate these issues while not causing
problems for applications.


>
>  From another quarter, an improvement I can envision is not re-running
> the pipeline on a commit that came from a merge request that already ran
> the pipeline right before merging. This is wasted work in 100% of cases.
>

Unfortunately as perverse as it seems there is a significant difference
between those pipelines - the ones running on the branch are running on a
protected branch and therefore have the power to publish binaries that are
picked up by other CI runs.
That is a cornerstone of how the CI system works and how changes to other
KDE projects are able to be made available almost immediately.

We can't privilege merge requests as we don't know that they're going to be
merged until they're actually merged, and you don't want to be keeping
binaries around unless you really need them (KWin is 1GB in size for each
platform for each build - because the CI system builds full debug binaries
with asserts enabled).


>
>
> Nate
>
>
Cheers,
Ben

Reply via email to