[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: CI system maintainability
From: Andrius_Štikonas <andrius () stikonas ! eu>
Date: 2019-03-29 11:42:49
Message-ID: FD34C4B7-241F-4563-83BF-20803476E8A9 () stikonas ! eu
[Download RAW message or body]
+1 for this. I think running tests before merging is more acceptable than \
having mandatory reviews.
On 29 March 2019 11:10:52 GMT+00:00, Ovidiu-Florin Bogdan \
<ovidiu.b13@gmail.com> wrote:
> Hello,
>
> A Merge Request in GitLab does not necessarily imply the need for a
> review by e person. It can just run a pipeline to validate that the
> code isn't broken. If the pipeline fails, the merge button is not
> available.
>
> We use GitLab at work and we have it set up like this:
>
> * Main branches (develop/master/release/etc) are proteted and cannot be
> directly commited/pushed to, and only updated through MR
> * Each project defines what it's build/validate pipeline is
> (Jenkinsfile in project repo)
> * The pipeline is executed uppon creating the MR
> * if the Pipeline passes, the MR can be merged to the mainline branch
>
> This way we ensure that no code gets in that fails the build or with
> tests failing.
>
> P.S. We also store the build artifacts in a binary repository from
> where other pipelines can fetch them to be used in compiling other
> projects.
>
> P.P.S. This is the "DevOps" process used in most companies. The tools
> might differ, but the process is the same. It's the same for most FOSS
> projects as well.
>
> Regards,
> Ovidiu
>
> În ziua de joi, 28 martie 2019, la 10:29:22 EET, Kevin Ottens a scris:
> > Hello,
> >
> > On Thursday, 28 March 2019 09:16:11 CET Ben Cooksley wrote:
> > > Please note that the commits in this instance were pushed without
> > > review, so restrictions on merge requests wouldn't make a
> difference
> > > in this case unfortunately.
> >
> > Maybe it's about time to make reviews mandatory... I know it's
> unpopular in
> > KDE, and I advocated for "don't force a tool if you can get someone
> to look at
> > your screen or pair with you" in the past. Clearly this compromise
> gets
> > somewhat exploited and that's especially bad in the case of a fragile
> and
> > central component like KDE PIM.
> >
> > Regards.
> >
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
[Attachment #3 (text/html)]
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.0//EN" \
"http://www.w3.org/TR/REC-html40/strict.dtd"><html><head><meta \
name="qrichtext" content="1"><style type="text/css"> p, li { white-space: \
pre-wrap; } </style></head><body style=" font-family:'Monospace'; \
font-size:10pt; font-weight:400; font-style:normal;">+1 for this. I think \
running tests before merging is more acceptable than having mandatory \
reviews.<br><br><div class="gmail_quote">On 29 March 2019 11:10:52 \
GMT+00:00, Ovidiu-Florin Bogdan <ovidiu.b13@gmail.com> \
wrote:<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; \
border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px;">Hello,</p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
"> </p> <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px;">A Merge Request in \
GitLab does not necessarily imply the need for a review by e person. It can \
just run a pipeline to validate that the code isn't broken. If the pipeline \
fails, the merge button is not available.</p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
"> </p> <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px;">We use GitLab at \
work and we have it set up like this:</p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
"> </p> <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px;">* Main branches \
(develop/master/release/etc) are proteted and cannot be directly \
commited/pushed to, and only updated through MR</p> <p style=" \
margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px;">* Each project defines what it's \
build/validate pipeline is (Jenkinsfile in project repo)</p> <p style=" \
margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px;">* The pipeline is executed uppon \
creating the MR</p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px;">* \
if the Pipeline passes, the MR can be merged to the mainline branch</p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
"> </p> <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px;">This way we ensure \
that no code gets in that fails the build or with tests failing.</p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
"> </p> <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px;">P.S. We also store \
the build artifacts in a binary repository from where other pipelines can \
fetch them to be used in compiling other projects.</p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
"> </p> <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px;">P.P.S. This is the \
"DevOps" process used in most companies. The tools might differ, but the \
process is the same. It's the same for most FOSS projects as well.</p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
"> </p> <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px;">Regards,</p> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px;">Ovidiu</p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
"> </p> <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px;">În ziua de joi, 28 \
martie 2019, la 10:29:22 EET, Kevin Ottens a scris:</p> <p style=" \
margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> Hello,</p> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">> </p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">> On Thursday, 28 March 2019 09:16:11 CET Ben \
Cooksley wrote:</p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">> > Please note that the commits in this instance \
were pushed without</p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">> > review, so restrictions on merge requests \
wouldn't make a difference</p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">> > in this case \
unfortunately.</p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">> </p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">> Maybe it's about time to make reviews mandatory... \
I know it's unpopular in </p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">> KDE, and I advocated for "don't force a tool if you \
can get someone to look at </p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">> your screen or pair with you" in \
the past. Clearly this compromise gets </p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">> somewhat exploited and that's \
especially bad in the case of a fragile and </p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">> central component like KDE \
PIM.</p> <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">> </p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">> Regards.</p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">> </p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
"> </p> <p style="-qt-paragraph-type:empty; margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; "> </p></blockquote></div><br>-- <br>Sent from my \
Android device with K-9 Mail. Please excuse my brevity.</body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic