Discussion:
[Pixman] Pushing unreviewed patches to the pixman git repository
Siarhei Siamashka
2018-06-06 01:06:13 UTC
Permalink
Hello,

I noticed that some people with commit access started pushing patches
to the pixman git repository without giving the pixman mailing list
subscribers any reasonable chance to review them:

https://cgit.freedesktop.org/pixman/commit/?id=8b95e0e460baa499e54c19d29bf761d34c25badc
https://cgit.freedesktop.org/pixman/commit/?id=bd2b49185b28c5024597a5e530af9fc25de3193a

Yes, these fixes were trivial. But still it would be more polite to
actually post patches to the mailing list, collect some reviews and
then *wait* at least severaldays before pushing them to the repository
(unless the issue is really urgent). Not everyone constantly monitors
the mailing list and is able to provide an instant response.

Thanks!
--
Best regards,
Siarhei Siamashka
Chris Wilson
2018-06-06 23:28:30 UTC
Permalink
Quoting Matt Turner (2018-06-06 23:55:39)
On Tue, Jun 5, 2018 at 6:06 PM, Siarhei Siamashka
Post by Siarhei Siamashka
Hello,
I noticed that some people with commit access started pushing patches
to the pixman git repository without giving the pixman mailing list
https://cgit.freedesktop.org/pixman/commit/?id=8b95e0e460baa499e54c19d29bf761d34c25badc
https://cgit.freedesktop.org/pixman/commit/?id=bd2b49185b28c5024597a5e530af9fc25de3193a
Yes, these fixes were trivial. But still it would be more polite to
actually post patches to the mailing list, collect some reviews and
then *wait* at least severaldays before pushing them to the repository
(unless the issue is really urgent). Not everyone constantly monitors
the mailing list and is able to provide an instant response.
I hope you don't consider those two patches to be similar cases.
One was committed without going to the mailing list by someone with
one patch in pixman every 5 years.
The other was was sent to the mailing list by a person with plenty of
pixman contributions and reviewed by two people. In Mesa we wait 24
hours, for the reasons you describe. Looks like it was close to 24
hours in this case.
I'm happy to wait more than 24 hours in the future -- that's no
problem. I'm just taking issue with the suggestion that the two cited
examples are somehow the same.
On a similar topic: now that we have the gitlab instance, how do we go
about enabling the reftests to be run automatically?
-Chris
Adam Jackson
2018-06-07 15:36:24 UTC
Permalink
Post by Chris Wilson
On a similar topic: now that we have the gitlab instance, how do we go
about enabling the reftests to be run automatically?
You push, and it happens:

https://gitlab.freedesktop.org/pixman/pixman/-/jobs

The change to enable this was unreviewed; mea maxima culpa. There's no
magic in it, at least:

---

commit 9034d0cc3241c56cbe3bdbc98247a68e3529ee48 (gitlab/ci, tmp, ci)
Author: Adam Jackson <***@redhat.com>
Date: Thu May 31 12:32:18 2018 -0400

ci: Add .gitlab-ci.yml

Just builds on Fedora 28 for x86_64 at the moment, but it's a start.
Credit to Daniel Stone for eliminating the nested docker image.

Signed-off-by: Adam Jackson <***@redhat.com>

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
new file mode 100644
index 0000000..b506ca3
--- /dev/null
+++ b/.gitlab-ci.yml
@@ -0,0 +1,9 @@
+image: fedora:28
+
+job:
+ script:
+ - dnf -y install dnf-plugins-core
+ - dnf -y groupinstall buildsys-build
+ - dnf -y builddep pixman
+ - ./autogen.sh
+ - make -sj4 check
\ No newline at end of file
diff --git a/contrib/ci.sh b/contrib/ci.sh
new file mode 100755
index 0000000..48b3e77
--- /dev/null
+++ b/contrib/ci.sh
@@ -0,0 +1,6 @@
+#!/bin/sh
+
+set -ex
+
+./autogen.sh
+make -sj4 check

---

D'oh, ought to call ci.sh from the script: section.

If you make your own fork of the repo, it'll automatically have CI
enabled too, so anything you push will get tested. One thing I wanted
to look into was getting enough of a qemu-user and cross-compiler
environment to be able to test on at least emulated non-x86.

- ajax

Loading...