You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@buildstream.apache.org by Tristan Van Berkom <tr...@codethink.co.uk> on 2022/10/26 14:54:33 UTC

[VOTE] Release buildstream / plugins 1.95.4 as 2.0

Hi all,

Please find below the proposed release tarballs and signatures:

  https://dist.apache.org/repos/dist/dev/buildstream/

I would like to call a VOTE over the next few days to release
these candidate tarballs BuildStream-1.95.4.dev0.tar.gz and
buildstream-plugins-1.95.3.tar.gz as 2.0.

[ ] +1: It's not just good, it's good enough!
[ ] +0: Let's have a talk.
[ ] -1: There's trouble in paradise. Here's what's wrong.

The computed digests of the tarballs up for vote are:


buildstream-plugins-1.95.3.tar.gz
---------------------------------
sha256: 2d33ed4cba762ccc09bbea060e089db08da5ce6150f903a03928da004dcaa387
sha512: ee22235884e7dfa54f40bd2baa2df1c26284ce19b4393310cd54dbf60b9789dd075eadacb3189b2002b3254025ed02129fc2e451cadd48ce9ff4da4e8de8a92d


BuildStream-1.95.4.dev0.tar.gz
------------------------------
sha256: 77f3aafa1268e4128108ac54fd6231cd5b548b0f2b00d84c9c83fc19f7095f60
sha512: 7cb335cc837cc70022ac398055e64c691863898daa2a9d0ae89270796b576e2ae692a2583c1a798cc34ba4769f73b92ff98ed26965f2ea2108df2c7ec490bc90


Cheers,
    -Tristan



Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0

Posted by Tristan Van Berkom <tr...@codethink.co.uk>.
Hi,

> On Oct 27, 2022, at 6:10 AM, Christofer Dutz <ch...@c-ware.de> wrote:
> 
> Hi,
> 
> as I was told that the 2.0 should be the first Apache release, will this then be the first Apache BuildStream release, or will this be one of the Non-Apache releases? I would assume, as you’re asking for votes, this is intended to be an Apache release, right?
> 

Yes,

Non-Apache releases are entirely informal, not voted on, and will be more clearly articulated in the future (as discussed with Sander on this list just yesterday).

Cheers,
    -Tristan


> Chris
> 
> 
> From: Tristan Van Berkom <tr...@codethink.co.uk>
> Date: Wednesday, 26. October 2022 at 16:55
> To: BuildStream <de...@buildstream.apache.org>
> Subject: [VOTE] Release buildstream / plugins 1.95.4 as 2.0
> Hi all,
> 
> Please find below the proposed release tarballs and signatures:
> 
>  https://dist.apache.org/repos/dist/dev/buildstream/
> 
> I would like to call a VOTE over the next few days to release
> these candidate tarballs BuildStream-1.95.4.dev0.tar.gz and
> buildstream-plugins-1.95.3.tar.gz as 2.0.
> 
> [ ] +1: It's not just good, it's good enough!
> [ ] +0: Let's have a talk.
> [ ] -1: There's trouble in paradise. Here's what's wrong.
> 
> The computed digests of the tarballs up for vote are:
> 
> 
> buildstream-plugins-1.95.3.tar.gz
> ---------------------------------
> sha256: 2d33ed4cba762ccc09bbea060e089db08da5ce6150f903a03928da004dcaa387
> sha512: ee22235884e7dfa54f40bd2baa2df1c26284ce19b4393310cd54dbf60b9789dd075eadacb3189b2002b3254025ed02129fc2e451cadd48ce9ff4da4e8de8a92d
> 
> 
> BuildStream-1.95.4.dev0.tar.gz
> ------------------------------
> sha256: 77f3aafa1268e4128108ac54fd6231cd5b548b0f2b00d84c9c83fc19f7095f60
> sha512: 7cb335cc837cc70022ac398055e64c691863898daa2a9d0ae89270796b576e2ae692a2583c1a798cc34ba4769f73b92ff98ed26965f2ea2108df2c7ec490bc90
> 
> 
> Cheers,
>    -Tristan
> 

Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0

Posted by Christofer Dutz <ch...@c-ware.de>.
Hi,

as I was told that the 2.0 should be the first Apache release, will this then be the first Apache BuildStream release, or will this be one of the Non-Apache releases? I would assume, as you’re asking for votes, this is intended to be an Apache release, right?

Chris


From: Tristan Van Berkom <tr...@codethink.co.uk>
Date: Wednesday, 26. October 2022 at 16:55
To: BuildStream <de...@buildstream.apache.org>
Subject: [VOTE] Release buildstream / plugins 1.95.4 as 2.0
Hi all,

Please find below the proposed release tarballs and signatures:

  https://dist.apache.org/repos/dist/dev/buildstream/

I would like to call a VOTE over the next few days to release
these candidate tarballs BuildStream-1.95.4.dev0.tar.gz and
buildstream-plugins-1.95.3.tar.gz as 2.0.

[ ] +1: It's not just good, it's good enough!
[ ] +0: Let's have a talk.
[ ] -1: There's trouble in paradise. Here's what's wrong.

The computed digests of the tarballs up for vote are:


buildstream-plugins-1.95.3.tar.gz
---------------------------------
sha256: 2d33ed4cba762ccc09bbea060e089db08da5ce6150f903a03928da004dcaa387
sha512: ee22235884e7dfa54f40bd2baa2df1c26284ce19b4393310cd54dbf60b9789dd075eadacb3189b2002b3254025ed02129fc2e451cadd48ce9ff4da4e8de8a92d


BuildStream-1.95.4.dev0.tar.gz
------------------------------
sha256: 77f3aafa1268e4128108ac54fd6231cd5b548b0f2b00d84c9c83fc19f7095f60
sha512: 7cb335cc837cc70022ac398055e64c691863898daa2a9d0ae89270796b576e2ae692a2583c1a798cc34ba4769f73b92ff98ed26965f2ea2108df2c7ec490bc90


Cheers,
    -Tristan


Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0

Posted by Christofer Dutz <ch...@c-ware.de>.
Hi Trisatan,

Unfortunately, I must disagree with your disagreement.

We have a bar that is set for projects being TLP at Apache and that bar is there for a reason.

As we can’t expect every project to instantly meet these expectations, that’s what we have the Apache Incubator for.
This is where projects get used to the voting process, how things are handled on the mailing-lists, how they get their releases into shape to meet the quality-bar of TLPs and so on.

When Perti was initiated, the promise was, that mature projects will be mentored under Pertri and its releases will meet the criteria as soon as they graduate. Admittedly I was a bit sceptical with this, and this situation seems to confirm this.

You have graduated and the same rules apply for BuildStream as they do for all other Apache TLP projects.

Sorry for having to be this clear.

Chris


From: Tristan Van Berkom <tr...@codethink.co.uk>
Date: Tuesday, 8. November 2022 at 08:26
To: dev@buildstream.apache.org <de...@buildstream.apache.org>
Subject: Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0
Hi Ben,

On Sat, 2022-11-05 at 15:25 +0000, Benjamin Schubert wrote:
> Hey everyone,
>
[...]
> -0
>
> I believe https://github.com/apache/buildstream/issues/1787 should be
> a blocker for this release, as it would otherwise negatively impact
> the first experience with it.

As you will notice I've currently closed this issue as I believe you're
not up to speed on the intentional behavioral change, which is to
ensure that after quitting a build, that any built elements (failed or
successful) continue to be pushed, but no new elements will be queued
to be built.

Also, I disagree with raising the bar this high for the initial 2.0
release: an initial 2.0 release is not necessarily the same as a "first
experience" with BuildStream 2.

Right now we've been struggling to cut through the red tape and get
used to the voting process and end up with a reasonable cadence for
subsequent releases (which is proving tiresome and more difficult than
expected); ensuring our release channel works and striking while the
iron is hot is a top priority for the project in my view at this time.

Right now we don't have any known issues which render the tool unusable
in any way, there may be some minor inconveniences or lack of polish in
some places, but nothing we cannot fix in a subsequent bugfix release
in an API stable fashion.

Cheers,
    -Tristan


Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0

Posted by Tristan Van Berkom <tr...@codethink.co.uk>.
Hi Ben,

On Sat, 2022-11-05 at 15:25 +0000, Benjamin Schubert wrote:
> Hey everyone,
> 
[...]
> -0
> 
> I believe https://github.com/apache/buildstream/issues/1787 should be
> a blocker for this release, as it would otherwise negatively impact
> the first experience with it.

As you will notice I've currently closed this issue as I believe you're
not up to speed on the intentional behavioral change, which is to
ensure that after quitting a build, that any built elements (failed or
successful) continue to be pushed, but no new elements will be queued
to be built.

Also, I disagree with raising the bar this high for the initial 2.0
release: an initial 2.0 release is not necessarily the same as a "first
experience" with BuildStream 2.

Right now we've been struggling to cut through the red tape and get
used to the voting process and end up with a reasonable cadence for
subsequent releases (which is proving tiresome and more difficult than
expected); ensuring our release channel works and striking while the
iron is hot is a top priority for the project in my view at this time.

Right now we don't have any known issues which render the tool unusable
in any way, there may be some minor inconveniences or lack of polish in
some places, but nothing we cannot fix in a subsequent bugfix release
in an API stable fashion.

Cheers,
    -Tristan



Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0

Posted by Christofer Dutz <ch...@c-ware.de>.
Hi Sander,

well … everyone is encouraged to vote … you don’t even have to be a Committer.

That’s why we have the “+1/0/-1” and “+1/0/-1 (binding)” as only binding votes count.
At least that’s what we teach projects in the incubator.
From: https://www.apache.org/foundation/voting.html
“PMC members have formally binding votes, but in general communities encourage all their members to vote, even if their votes are only advisory.“
And we generally count everyone interested in a project as that project’s community.

Also, one of the checks we usually do for incubator releases, is that there’s an “apache-“ prefix on the release artifacts.
But I did mark it as “MINOR”. But I did click though the repo for other projects and just clicking though the directories starting with “a” more than 90% seem to be prefixing their source bundles with “apache-“. It does however seem, that especially the ancient ASF projects don’t quite seem to follow this strategy, but the newer ones seem to all to it the same way.

If there’s no KEY anywhere, I cannot validate the signature. If I can’t validate the signature technically nobody can vote +1.

Yes … most of the invalid license headers (Or missing ones) refer to test and documentation, however in my understanding these files too should have the headers in place.

Having a look at the amhello.tar.gz again. The “compile” script has this header text:
# Copyright (C) 1999-2014 Free Software Foundation, Inc.
# Written by Tom Tromey <tr...@cygnus.com>.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 2, or (at your option)
# any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program.  If not, see <http://www.gnu.org/licenses/>.

So, what I wanted to point out, is that we generally expect people, especially those with binding votes (members of the PMC), to do these checks, that I did.

Chris



From: Sander Striker <s....@striker.nl>
Date: Sunday, 6. November 2022 at 14:50
To: dev@buildstream.apache.org <de...@buildstream.apache.org>
Subject: Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0
Hi Chris,

On Sun, Nov 6, 2022 at 11:15 AM Christofer Dutz <ch...@c-ware.de>
wrote:

> Sorry I must do this … but …
>
> -1  (Chirs)
>

To avoid confusion, posting your feedback in the future without adding a
"-1" is more helpful and appropriate such that it is not misconstrued as an
actual vote.


> [MINOR] Download all staged artifacts under the url specified in the
> release vote email.
>
>   *   Generally, we like our download artifacts to be prefixed with
> “apache-“
>

https://dist.apache.org/repos/dist/release/httpd/ as a counter example.
That said, I actually see a benefit to the apache- prefix here, given the
non ASF maintenance releases.  It will make it easier to distinguish.


>   *   Most projects generally use a {version}/{rc}/ directory structure
> with a KEYS file in the projects root
> [FAILED] Verify the signature is correct.
>

I'm expecting us to put a KEYS file here
https://dist.apache.org/repos/dist/release/buildstream/.


>   *   No KEYS file containing the public signatures of the release-manager
> used to sign the release
>   *   Couldn’t find key on any public servers I searched

[OK] Check if the signature references an Apache email address.
> [OK] Verify the SHA512 hashes.
>
>   *   Both Hashes match
> [OK] Unpack the archive.
> [OK] Verify the existence of LICENSE, NOTICE files in the extracted source
> bundle.
> [MINOR] Verify the content of LICENSE, NOTICE files in the extracted
> source bundle.
>
>   *   The NOTICE file of the plugins archive references 2021
> [FAILED] [RM] Run RAT externally to ensure there are no surprises.
>
>   *   Main bundle:
>      *   1924 Unknown Licenses for the main bundle (Attached as rat.txt)
>

The bulk seems to be tests and docs?  The docs should be more easily
addressable.


>      *   Some sources seem to be GPL licensed:
>         *
>  BuildStream-1.95.4.dev0/src/buildstream/_scheduler/queues/cachequeryqueue.py
>

That's a good catch - this one seems to have slipped through the cracks
back when the changes landed in January after iterating on them from
September 2021.


>      *   Some sources don’t seem to be having any header:
>         *   BuildStream-1.95.4.dev0/src/buildstream/_scheduler/resources.py
>

I can see how this one happened, it didn't have a header since it was
introduced in a refactoring in 2018.  Good catch as well.


>      *   Tests/integration/project/files/amhello.tar.gz (all other copies
> of this file too) is a binary file (which is generally not allowed) and
> contains GPL licensed content and is infringing that license by not
> distributing the license with it (which is even less allowed).
>

Note that these files all note "This program is free software; the Free
Software Foundation gives unlimited permission to copy, distribute and
modify it.", in line with generated autotools files.  That said, we could
have it not be in a tarball, and also given it is a test, we may be able to
download it at test time.


>      *   Admittedly I stopped a detailed analysis of other problems as
> this is already enough for a -1
>

Your input is appreciated.


>   *   Plugin bundle:
>      *   Rat reports: 17 Unknown Licenses for the plugin bundle (Attached
> as rat-plugin.txt)
> [OK] Search for Copyright references, and if they are in headers, make
> sure these files containing them are mentioned in the LICENSE file.
>

I see an .asf.yaml file which we don't need to distribute.  There's 2 empty
__init__.py files, 3 *_requirements.txt files.  The egg-info directories
are generated.
The setup.cfg file seems to have lost its header in the packaging process,
as it is there in the source repo.
The PKG-INFO file actually needs its Authors reference updated.


> I’ve uploaded the rat.log and rat-plugin.log here:
> https://drive.google.com/drive/folders/1FaQj8TZbH3XMXxEvpEPazOGFd9L0rL4z?usp=sharing


Thanks again.

Cheers,

Sander

From: Benjamin Schubert <co...@benschubert.me>
> Date: Saturday, 5. November 2022 at 16:26
> To: dev@buildstream.apache.org <de...@buildstream.apache.org>
> Subject: Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0
> Hey everyone,
>
> > Le mer. 26 oct. 2022 à 15:55, Tristan Van Berkom
> > tristan.vanberkom@codethink.co.uk a écrit :
> >
> > > buildstream-plugins-1.95.3.tar.gz
> > > ---------------------------------
> > > sha256:
> 2d33ed4cba762ccc09bbea060e089db08da5ce6150f903a03928da004dcaa387
> > > sha512:
> ee22235884e7dfa54f40bd2baa2df1c26284ce19b4393310cd54dbf60b9789dd075eadacb3189b2002b3254025ed02129fc2e451cadd48ce9ff4da4e8de8a92d
> > >
> > > BuildStream-1.95.4.dev0.tar.gz
> > > ------------------------------
> > > sha256:
> 77f3aafa1268e4128108ac54fd6231cd5b548b0f2b00d84c9c83fc19f7095f60
> > > sha512:
> 7cb335cc837cc70022ac398055e64c691863898daa2a9d0ae89270796b576e2ae692a2583c1a798cc34ba4769f73b92ff98ed26965f2ea2108df2c7ec490bc90
> >
>
> -0
>
> I believe https://github.com/apache/buildstream/issues/1787 should be a
> blocker for this release, as it would otherwise negatively impact the first
> experience with it.
>

Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0

Posted by Rich Bowen <rb...@apache.org>.
You are absolutely correct, and I apologize for my very brusque tone in my earlier email. I don't want, in any way, to make you feel unwelcome here. Please forgive me.

To be completely rules-bound, the general rule is that in formal communication (blog posts, press releases, and so on) it's "Apache Foo" on the first mention, and just "Foo" thereafter.

But you are completely correct, actual humans will always, only, ever, call it BuildStream and that's totally fine.

On 2022/11/09 06:24:55 Tristan Van Berkom wrote:
> Hi,
> 
> On Tue, 2022-11-08 at 14:27 +0000, Rich Bowen wrote:
> > I find this specific comment very odd, and wonder where communication
> > has broken down.
> > 
> > > I have reluctantly conceded to use "Apache BuildStream" in the
> > > release
> > > announcements in order to make this distinction, however, I want to
> > > be
> > > very clear that I am against rebranding of BuildStream, just
> > > because we
> > > are now under the Apache umbrella.
> > 
> > When a project comes to Apache, it gets rebranded to Apache Foo.
> > That's how we communicate to the world that it's an Apache project.
> > This is a consistent, and mandatory, part of bringing a project to
> > the Foundation. The Foundation exists, in part, to protect our
> > brands. When BuildStream was established as a TLP, it became Apache
> > BuildStream. That's not something that we should be debating, at this
> > late stage.
> 
> Apparently Sander and yourself are in agreement about this, but indeed
> it would have been good to be more clear about this when starting out
> on this path.
> 
> As I understand it, I'm sure in some places policy will dictate that we
> use the term "Apache BuildStream", but I'm sure that whatever is
> written, human beings will still generally refer to the project as
> "BuildStream", that is I'm sure the case for any project under the
> Apache umbrella (except for perhaps httpd, which is simply called
> "Apache" by most humans).
> 
> Consider this point conceded, however I am still asking for some leeway
> in this - I don't want us to be taking every opportunity to shout the
> fact that we are now an Apache project to our users.
> 
> As there is always a vocal minority of ideologues on either side of the
> Free/Open Source Software debate - I think we should at the very least
> be sensitive to the fact that at least some of our user base may not be
> happy with the relicensing.
> 
> We were never "GNOME BuildStream", and being as vocal as possible about
> being "Apache BuildStream" now is just insensitive and IMO at least a
> bit rude - our community and users already know, and there is no need
> to rub salt in old wounds.
> 
> 
> On Tue, 2022-11-08 at 15:37 +0100, Sander Striker wrote:
> [...]
> > I am going to be more insistent on this.  The apache- prefix serves a
> > real purpose here,
> > even more so than for projects that have been at the ASF since
> > inception: it clearly separates Apache BuildStream releases from
> > BuildStream maintenance releases.
> 
> I think renaming the tarball in the main official distribution channel
> here is just an opportunity to be exorbitantly vocal about BuildStream
> having moved to Apache, and as far as I can tell by the language
> expressed so far, it is entirely unnecessary, just "preferred".
> 
> In my view, the fact that the tarball appears on dist.apache.org should
> be enough of a distinction here.
> 
> 
> Sander, you might have the power to veto this - but I'm asking that you
> just take a pause first, this is just the name of the tarball here.
> 
> Cheers,
>     -Tristan
> 
> 
> 

Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0

Posted by Tristan Van Berkom <tr...@codethink.co.uk>.
Hi,

On Tue, 2022-11-08 at 14:27 +0000, Rich Bowen wrote:
> I find this specific comment very odd, and wonder where communication
> has broken down.
> 
> > I have reluctantly conceded to use "Apache BuildStream" in the
> > release
> > announcements in order to make this distinction, however, I want to
> > be
> > very clear that I am against rebranding of BuildStream, just
> > because we
> > are now under the Apache umbrella.
> 
> When a project comes to Apache, it gets rebranded to Apache Foo.
> That's how we communicate to the world that it's an Apache project.
> This is a consistent, and mandatory, part of bringing a project to
> the Foundation. The Foundation exists, in part, to protect our
> brands. When BuildStream was established as a TLP, it became Apache
> BuildStream. That's not something that we should be debating, at this
> late stage.

Apparently Sander and yourself are in agreement about this, but indeed
it would have been good to be more clear about this when starting out
on this path.

As I understand it, I'm sure in some places policy will dictate that we
use the term "Apache BuildStream", but I'm sure that whatever is
written, human beings will still generally refer to the project as
"BuildStream", that is I'm sure the case for any project under the
Apache umbrella (except for perhaps httpd, which is simply called
"Apache" by most humans).

Consider this point conceded, however I am still asking for some leeway
in this - I don't want us to be taking every opportunity to shout the
fact that we are now an Apache project to our users.

As there is always a vocal minority of ideologues on either side of the
Free/Open Source Software debate - I think we should at the very least
be sensitive to the fact that at least some of our user base may not be
happy with the relicensing.

We were never "GNOME BuildStream", and being as vocal as possible about
being "Apache BuildStream" now is just insensitive and IMO at least a
bit rude - our community and users already know, and there is no need
to rub salt in old wounds.


On Tue, 2022-11-08 at 15:37 +0100, Sander Striker wrote:
[...]
> I am going to be more insistent on this.  The apache- prefix serves a
> real purpose here,
> even more so than for projects that have been at the ASF since
> inception: it clearly separates Apache BuildStream releases from
> BuildStream maintenance releases.

I think renaming the tarball in the main official distribution channel
here is just an opportunity to be exorbitantly vocal about BuildStream
having moved to Apache, and as far as I can tell by the language
expressed so far, it is entirely unnecessary, just "preferred".

In my view, the fact that the tarball appears on dist.apache.org should
be enough of a distinction here.


Sander, you might have the power to veto this - but I'm asking that you
just take a pause first, this is just the name of the tarball here.

Cheers,
    -Tristan



Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0

Posted by Rich Bowen <rb...@apache.org>.
I find this specific comment very odd, and wonder where communication has broken down.

> I have reluctantly conceded to use "Apache BuildStream" in the release
> announcements in order to make this distinction, however, I want to be
> very clear that I am against rebranding of BuildStream, just because we
> are now under the Apache umbrella.

When a project comes to Apache, it gets rebranded to Apache Foo. That's how we communicate to the world that it's an Apache project. This is a consistent, and mandatory, part of bringing a project to the Foundation. The Foundation exists, in part, to protect our brands. When BuildStream was established as a TLP, it became Apache BuildStream. That's not something that we should be debating, at this late stage.

Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0

Posted by Sander Striker <s....@striker.nl>.
Hi,

On Tue, Nov 8, 2022 at 9:19 AM Tristan Van Berkom <
tristan.vanberkom@codethink.co.uk> wrote:

> Hi,
>
> Sorry for the lag I have other urgent projects to deal with as do we
> all I'm sure :)
>
> Thanks again Christofer for taking an interest, I'm sure some of your
> points are indeed red flags and it's good to catch them.
>
>
> Replies inline...
>
> On Sun, 2022-11-06 at 14:50 +0100, Sander Striker wrote:
> > Hi Chris,
> >
> > On Sun, Nov 6, 2022 at 11:15 AM Christofer Dutz <
> christofer.dutz@c-ware.de>
> > wrote:
> >
> > > Sorry I must do this … but …
> > >
> > > -1  (Chirs)
> > >
> >
> > To avoid confusion, posting your feedback in the future without adding a
> > "-1" is more helpful and appropriate such that it is not misconstrued as
> an
> > actual vote.
> >
> >
> > > [MINOR] Download all staged artifacts under the url specified in the
> > > release vote email.
> > >
> > >   *   Generally, we like our download artifacts to be prefixed with
> > > “apache-“
> > >
> >
> > https://dist.apache.org/repos/dist/release/httpd/ as a counter example.
> > That said, I actually see a benefit to the apache- prefix here, given the
> > non ASF maintenance releases.  It will make it easier to distinguish.
>
> I have reluctantly conceded to use "Apache BuildStream" in the release
> announcements in order to make this distinction, however, I want to be
> very clear that I am against rebranding of BuildStream, just because we
> are now under the Apache umbrella.
>

I'm sorry you feel this way Tristan, if there is any disconnect here it's
on me.
When we started down the road of going into the ASF, the destination was
always "Apache BuildStream", just like any other ASF project.


> I don't think this distinction is particularly useful seeing that a
> maintenance 1.6 support release will never appear on dist.apache.org at
> all.
>
> So I think no, there is no need for our release channel to be
> publishing "apache-" prefixed source tarballs.
>

I am going to be more insistent on this.  The apache- prefix serves a real
purpose here,
even more so than for projects that have been at the ASF since inception:
it clearly
separates Apache BuildStream releases from BuildStream maintenance releases.

> >   *   Most projects generally use a {version}/{rc}/ directory structure
> > > with a KEYS file in the projects root
> > > [FAILED] Verify the signature is correct.
> > >
> >
> > I'm expecting us to put a KEYS file here
> > https://dist.apache.org/repos/dist/release/buildstream/.
> >
>
> Ok, I haven't seen this in the httpd example or in release process
> documentation.
>
> Sander: please spend time with me once the vote finally passes to
> ensure that we are meeting expectations regarding this detail.
>

Happy to do so as always.

> >   *   No KEYS file containing the public signatures of the
> release-manager
> > > used to sign the release
> > >   *   Couldn’t find key on any public servers I searched
> >
> > [OK] Check if the signature references an Apache email address.
> > > [OK] Verify the SHA512 hashes.
> > >
> > >   *   Both Hashes match
> > > [OK] Unpack the archive.
> > > [OK] Verify the existence of LICENSE, NOTICE files in the extracted
> source
> > > bundle.
> > > [MINOR] Verify the content of LICENSE, NOTICE files in the extracted
> > > source bundle.
> > >
> > >   *   The NOTICE file of the plugins archive references 2021
> > > [FAILED] [RM] Run RAT externally to ensure there are no surprises.
> > >
>
> Thanks, lets take an action on this :)
>
> > >   *   Main bundle:
> > >      *   1924 Unknown Licenses for the main bundle (Attached as
> rat.txt)
> > >
> >
> > The bulk seems to be tests and docs?  The docs should be more easily
> > addressable.
>
> To be clear, what is expected here ?
>
> Do we want to add license headings to all of the .rst files comprising
> the documentation sources ?
>
> I suppose I'll do this anyway, perhaps I can script it.
>
> >
> > >      *   Some sources seem to be GPL licensed:
> > >         *
> > >
> BuildStream-1.95.4.dev0/src/buildstream/_scheduler/queues/cachequeryqueue.py
> > >
> >
> > That's a good catch - this one seems to have slipped through the cracks
> > back when the changes landed in January after iterating on them from
> > September 2021.
>
> Another action point, I'm not sure how this escaped grep :)
>
> > >      *   Some sources don’t seem to be having any header:
> > >         *
>  BuildStream-1.95.4.dev0/src/buildstream/_scheduler/resources.py
> > >
> >
> > I can see how this one happened, it didn't have a header since it was
> > introduced in a refactoring in 2018.  Good catch as well.
>
> Another action point.
>
> >
> > >      *   Tests/integration/project/files/amhello.tar.gz (all other
> copies
> > > of this file too) is a binary file (which is generally not allowed) and
> > > contains GPL licensed content and is infringing that license by not
> > > distributing the license with it (which is even less allowed).
> > >
> >
> > Note that these files all note "This program is free software; the Free
> > Software Foundation gives unlimited permission to copy, distribute and
> > modify it.", in line with generated autotools files.  That said, we could
> > have it not be in a tarball, and also given it is a test, we may be able
> to
> > download it at test time.
> >
>
> As Christofer pointed out, we have accidentally included some autotools
> generated files which are generated at build time, which include the
> GPL license heading.
>
> However, the autotools example itself is not GPL licensed and as stated
> above, the FSF gives unlimited permission to copy, distribute and
> modify this content.
>
> The content of these tarballs *should* only contain the files here:
>
>     https://git.savannah.gnu.org/cgit/automake.git/tree/doc/amhello
>
> I will amend these tarballs (which are all in fact the same tiny
> tarball repeated in multiple test directories) to not include the
> unneeded generated GPL licensed files.
>
> I am not really amenable to refactoring the test suite to download
> these tarballs just for this detail, it's overkill. If it was the case
> that we did in fact require GPL code for these tests, I would be more
> happy to rewrite the content of these tarballs from scratch than to
> work out downloading these at test time.
>

Let's choose a path of least resistance that covers the needs.


> > >      *   Admittedly I stopped a detailed analysis of other problems
> > > as
> > > this is already enough for a -1
> > >
> >
> > Your input is appreciated.
> >
> >
> > >   *   Plugin bundle:
> > >      *   Rat reports: 17 Unknown Licenses for the plugin bundle
> > > (Attached
> > > as rat-plugin.txt)
> > > [OK] Search for Copyright references, and if they are in headers,
> > > make
> > > sure these files containing them are mentioned in the LICENSE file.
> > >
> >
> > I see an .asf.yaml file which we don't need to distribute.  There's 2
> > empty
> > __init__.py files, 3 *_requirements.txt files.  The egg-info
> > directories
> > are generated.
> > The setup.cfg file seems to have lost its header in the packaging
> > process,
> > as it is there in the source repo.
> > The PKG-INFO file actually needs its Authors reference updated.
> >
>
> Looking at this, the Author field already says "BuildStream Developers"
> which accurately reflects what is specified in setup.py.
>
> The Home-page, Author-email and License all also appear to be correct.
>
> However, some of the Project-URL entries have not yet been changed to
> point to github instead of gitlab.
>

Indeed, so let's get these updated as well.


> What would you expect to be listed as the "Author" field instead ?
>

"The Apache Software Foundation" - think of the author as the entity
releasing the software.

The Apache Airflow project can actually serve as a nice example:
https://github.com/apache/airflow/blob/main/setup.cfg


> Action Summary
> ==============
> Here are the actions I intend to take for yet another try to get our
> release channel up and running:
>
>  * Update Copyright year in buildstream-plugins NOTICE file
>
>  * Fix inaccurate license heading in:
>    src/buildstream/_scheduler/queues/cachequeryqueue.py
>
>  * Add missing license heading in:
>    src/buildstream/_scheduler/resources.py
>
>  * Update remnant gitlab links in setup.py (and consequently PKG-INFO)
>
>  * Regenerate the amhello.tar.gz tarballs to accurately reflect the
>    public domain material offered by upstream autotools:
>
>    https://git.savannah.gnu.org/cgit/automake.git/tree/doc/amhello
>
>  * Add license headings to all of the documentation sources.
>

Cheers,

Sander


> Cheers,
>     -Tristan
>
>
>

Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0

Posted by Tristan Van Berkom <tr...@codethink.co.uk>.
Hi,

Sorry for the lag I have other urgent projects to deal with as do we
all I'm sure :)

Thanks again Christofer for taking an interest, I'm sure some of your
points are indeed red flags and it's good to catch them.


Replies inline...

On Sun, 2022-11-06 at 14:50 +0100, Sander Striker wrote:
> Hi Chris,
> 
> On Sun, Nov 6, 2022 at 11:15 AM Christofer Dutz <ch...@c-ware.de>
> wrote:
> 
> > Sorry I must do this … but …
> > 
> > -1  (Chirs)
> > 
> 
> To avoid confusion, posting your feedback in the future without adding a
> "-1" is more helpful and appropriate such that it is not misconstrued as an
> actual vote.
> 
> 
> > [MINOR] Download all staged artifacts under the url specified in the
> > release vote email.
> > 
> >   *   Generally, we like our download artifacts to be prefixed with
> > “apache-“
> > 
> 
> https://dist.apache.org/repos/dist/release/httpd/ as a counter example.
> That said, I actually see a benefit to the apache- prefix here, given the
> non ASF maintenance releases.  It will make it easier to distinguish.

I have reluctantly conceded to use "Apache BuildStream" in the release
announcements in order to make this distinction, however, I want to be
very clear that I am against rebranding of BuildStream, just because we
are now under the Apache umbrella.

I don't think this distinction is particularly useful seeing that a
maintenance 1.6 support release will never appear on dist.apache.org at
all.

So I think no, there is no need for our release channel to be
publishing "apache-" prefixed source tarballs.


> >   *   Most projects generally use a {version}/{rc}/ directory structure
> > with a KEYS file in the projects root
> > [FAILED] Verify the signature is correct.
> > 
> 
> I'm expecting us to put a KEYS file here
> https://dist.apache.org/repos/dist/release/buildstream/.
> 

Ok, I haven't seen this in the httpd example or in release process
documentation.

Sander: please spend time with me once the vote finally passes to
ensure that we are meeting expectations regarding this detail.


> >   *   No KEYS file containing the public signatures of the release-manager
> > used to sign the release
> >   *   Couldn’t find key on any public servers I searched
> 
> [OK] Check if the signature references an Apache email address.
> > [OK] Verify the SHA512 hashes.
> > 
> >   *   Both Hashes match
> > [OK] Unpack the archive.
> > [OK] Verify the existence of LICENSE, NOTICE files in the extracted source
> > bundle.
> > [MINOR] Verify the content of LICENSE, NOTICE files in the extracted
> > source bundle.
> > 
> >   *   The NOTICE file of the plugins archive references 2021
> > [FAILED] [RM] Run RAT externally to ensure there are no surprises.
> > 

Thanks, lets take an action on this :)

> >   *   Main bundle:
> >      *   1924 Unknown Licenses for the main bundle (Attached as rat.txt)
> > 
> 
> The bulk seems to be tests and docs?  The docs should be more easily
> addressable.

To be clear, what is expected here ?

Do we want to add license headings to all of the .rst files comprising
the documentation sources ?

I suppose I'll do this anyway, perhaps I can script it.

> 
> >      *   Some sources seem to be GPL licensed:
> >         *
> >  BuildStream-1.95.4.dev0/src/buildstream/_scheduler/queues/cachequeryqueue.py
> > 
> 
> That's a good catch - this one seems to have slipped through the cracks
> back when the changes landed in January after iterating on them from
> September 2021.

Another action point, I'm not sure how this escaped grep :)

> >      *   Some sources don’t seem to be having any header:
> >         *   BuildStream-1.95.4.dev0/src/buildstream/_scheduler/resources.py
> > 
> 
> I can see how this one happened, it didn't have a header since it was
> introduced in a refactoring in 2018.  Good catch as well.

Another action point.

> 
> >      *   Tests/integration/project/files/amhello.tar.gz (all other copies
> > of this file too) is a binary file (which is generally not allowed) and
> > contains GPL licensed content and is infringing that license by not
> > distributing the license with it (which is even less allowed).
> > 
> 
> Note that these files all note "This program is free software; the Free
> Software Foundation gives unlimited permission to copy, distribute and
> modify it.", in line with generated autotools files.  That said, we could
> have it not be in a tarball, and also given it is a test, we may be able to
> download it at test time.
> 

As Christofer pointed out, we have accidentally included some autotools
generated files which are generated at build time, which include the
GPL license heading.

However, the autotools example itself is not GPL licensed and as stated
above, the FSF gives unlimited permission to copy, distribute and
modify this content.

The content of these tarballs *should* only contain the files here:

    https://git.savannah.gnu.org/cgit/automake.git/tree/doc/amhello

I will amend these tarballs (which are all in fact the same tiny
tarball repeated in multiple test directories) to not include the
unneeded generated GPL licensed files.

I am not really amenable to refactoring the test suite to download
these tarballs just for this detail, it's overkill. If it was the case
that we did in fact require GPL code for these tests, I would be more
happy to rewrite the content of these tarballs from scratch than to
work out downloading these at test time.


> >      *   Admittedly I stopped a detailed analysis of other problems
> > as
> > this is already enough for a -1
> > 
> 
> Your input is appreciated.
> 
> 
> >   *   Plugin bundle:
> >      *   Rat reports: 17 Unknown Licenses for the plugin bundle
> > (Attached
> > as rat-plugin.txt)
> > [OK] Search for Copyright references, and if they are in headers,
> > make
> > sure these files containing them are mentioned in the LICENSE file.
> > 
> 
> I see an .asf.yaml file which we don't need to distribute.  There's 2
> empty
> __init__.py files, 3 *_requirements.txt files.  The egg-info
> directories
> are generated.
> The setup.cfg file seems to have lost its header in the packaging
> process,
> as it is there in the source repo.
> The PKG-INFO file actually needs its Authors reference updated.
> 

Looking at this, the Author field already says "BuildStream Developers"
which accurately reflects what is specified in setup.py.

The Home-page, Author-email and License all also appear to be correct.

However, some of the Project-URL entries have not yet been changed to
point to github instead of gitlab.

What would you expect to be listed as the "Author" field instead ?


Action Summary
==============
Here are the actions I intend to take for yet another try to get our
release channel up and running:

 * Update Copyright year in buildstream-plugins NOTICE file

 * Fix inaccurate license heading in:
   src/buildstream/_scheduler/queues/cachequeryqueue.py

 * Add missing license heading in:
   src/buildstream/_scheduler/resources.py

 * Update remnant gitlab links in setup.py (and consequently PKG-INFO)

 * Regenerate the amhello.tar.gz tarballs to accurately reflect the
   public domain material offered by upstream autotools:

   https://git.savannah.gnu.org/cgit/automake.git/tree/doc/amhello

 * Add license headings to all of the documentation sources.


Cheers,
    -Tristan



Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0

Posted by Sander Striker <s....@striker.nl>.
Hi Chris,

On Sun, Nov 6, 2022 at 11:15 AM Christofer Dutz <ch...@c-ware.de>
wrote:

> Sorry I must do this … but …
>
> -1  (Chirs)
>

To avoid confusion, posting your feedback in the future without adding a
"-1" is more helpful and appropriate such that it is not misconstrued as an
actual vote.


> [MINOR] Download all staged artifacts under the url specified in the
> release vote email.
>
>   *   Generally, we like our download artifacts to be prefixed with
> “apache-“
>

https://dist.apache.org/repos/dist/release/httpd/ as a counter example.
That said, I actually see a benefit to the apache- prefix here, given the
non ASF maintenance releases.  It will make it easier to distinguish.


>   *   Most projects generally use a {version}/{rc}/ directory structure
> with a KEYS file in the projects root
> [FAILED] Verify the signature is correct.
>

I'm expecting us to put a KEYS file here
https://dist.apache.org/repos/dist/release/buildstream/.


>   *   No KEYS file containing the public signatures of the release-manager
> used to sign the release
>   *   Couldn’t find key on any public servers I searched

[OK] Check if the signature references an Apache email address.
> [OK] Verify the SHA512 hashes.
>
>   *   Both Hashes match
> [OK] Unpack the archive.
> [OK] Verify the existence of LICENSE, NOTICE files in the extracted source
> bundle.
> [MINOR] Verify the content of LICENSE, NOTICE files in the extracted
> source bundle.
>
>   *   The NOTICE file of the plugins archive references 2021
> [FAILED] [RM] Run RAT externally to ensure there are no surprises.
>
>   *   Main bundle:
>      *   1924 Unknown Licenses for the main bundle (Attached as rat.txt)
>

The bulk seems to be tests and docs?  The docs should be more easily
addressable.


>      *   Some sources seem to be GPL licensed:
>         *
>  BuildStream-1.95.4.dev0/src/buildstream/_scheduler/queues/cachequeryqueue.py
>

That's a good catch - this one seems to have slipped through the cracks
back when the changes landed in January after iterating on them from
September 2021.


>      *   Some sources don’t seem to be having any header:
>         *   BuildStream-1.95.4.dev0/src/buildstream/_scheduler/resources.py
>

I can see how this one happened, it didn't have a header since it was
introduced in a refactoring in 2018.  Good catch as well.


>      *   Tests/integration/project/files/amhello.tar.gz (all other copies
> of this file too) is a binary file (which is generally not allowed) and
> contains GPL licensed content and is infringing that license by not
> distributing the license with it (which is even less allowed).
>

Note that these files all note "This program is free software; the Free
Software Foundation gives unlimited permission to copy, distribute and
modify it.", in line with generated autotools files.  That said, we could
have it not be in a tarball, and also given it is a test, we may be able to
download it at test time.


>      *   Admittedly I stopped a detailed analysis of other problems as
> this is already enough for a -1
>

Your input is appreciated.


>   *   Plugin bundle:
>      *   Rat reports: 17 Unknown Licenses for the plugin bundle (Attached
> as rat-plugin.txt)
> [OK] Search for Copyright references, and if they are in headers, make
> sure these files containing them are mentioned in the LICENSE file.
>

I see an .asf.yaml file which we don't need to distribute.  There's 2 empty
__init__.py files, 3 *_requirements.txt files.  The egg-info directories
are generated.
The setup.cfg file seems to have lost its header in the packaging process,
as it is there in the source repo.
The PKG-INFO file actually needs its Authors reference updated.


> I’ve uploaded the rat.log and rat-plugin.log here:
> https://drive.google.com/drive/folders/1FaQj8TZbH3XMXxEvpEPazOGFd9L0rL4z?usp=sharing


Thanks again.

Cheers,

Sander

From: Benjamin Schubert <co...@benschubert.me>
> Date: Saturday, 5. November 2022 at 16:26
> To: dev@buildstream.apache.org <de...@buildstream.apache.org>
> Subject: Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0
> Hey everyone,
>
> > Le mer. 26 oct. 2022 à 15:55, Tristan Van Berkom
> > tristan.vanberkom@codethink.co.uk a écrit :
> >
> > > buildstream-plugins-1.95.3.tar.gz
> > > ---------------------------------
> > > sha256:
> 2d33ed4cba762ccc09bbea060e089db08da5ce6150f903a03928da004dcaa387
> > > sha512:
> ee22235884e7dfa54f40bd2baa2df1c26284ce19b4393310cd54dbf60b9789dd075eadacb3189b2002b3254025ed02129fc2e451cadd48ce9ff4da4e8de8a92d
> > >
> > > BuildStream-1.95.4.dev0.tar.gz
> > > ------------------------------
> > > sha256:
> 77f3aafa1268e4128108ac54fd6231cd5b548b0f2b00d84c9c83fc19f7095f60
> > > sha512:
> 7cb335cc837cc70022ac398055e64c691863898daa2a9d0ae89270796b576e2ae692a2583c1a798cc34ba4769f73b92ff98ed26965f2ea2108df2c7ec490bc90
> >
>
> -0
>
> I believe https://github.com/apache/buildstream/issues/1787 should be a
> blocker for this release, as it would otherwise negatively impact the first
> experience with it.
>

Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0

Posted by Christofer Dutz <ch...@c-ware.de>.
Sorry I must do this … but …

-1  (Chirs)


[MINOR] Download all staged artifacts under the url specified in the release vote email.

  *   Generally, we like our download artifacts to be prefixed with “apache-“
  *   Most projects generally use a {version}/{rc}/ directory structure with a KEYS file in the projects root
[FAILED] Verify the signature is correct.

  *   No KEYS file containing the public signatures of the release-manager used to sign the release
  *   Couldn’t find key on any public servers I searched
[OK] Check if the signature references an Apache email address.
[OK] Verify the SHA512 hashes.

  *   Both Hashes match
[OK] Unpack the archive.
[OK] Verify the existence of LICENSE, NOTICE files in the extracted source bundle.
[MINOR] Verify the content of LICENSE, NOTICE files in the extracted source bundle.

  *   The NOTICE file of the plugins archive references 2021
[FAILED] [RM] Run RAT externally to ensure there are no surprises.

  *   Main bundle:
     *   1924 Unknown Licenses for the main bundle (Attached as rat.txt)
     *   Some sources seem to be GPL licensed:
        *   BuildStream-1.95.4.dev0/src/buildstream/_scheduler/queues/cachequeryqueue.py
     *   Some sources don’t seem to be having any header:
        *   BuildStream-1.95.4.dev0/src/buildstream/_scheduler/resources.py
     *   Tests/integration/project/files/amhello.tar.gz (all other copies of this file too) is a binary file (which is generally not allowed) and contains GPL licensed content and is infringing that license by not distributing the license with it (which is even less allowed).
     *   Admittedly I stopped a detailed analysis of other problems as this is already enough for a -1
  *   Plugin bundle:
     *   Rat reports: 17 Unknown Licenses for the plugin bundle (Attached as rat-plugin.txt)
[OK] Search for Copyright references, and if they are in headers, make sure these files containing them are mentioned in the LICENSE file.

I’ve uploaded the rat.log and rat-plugin.log here: https://drive.google.com/drive/folders/1FaQj8TZbH3XMXxEvpEPazOGFd9L0rL4z?usp=sharing


From: Benjamin Schubert <co...@benschubert.me>
Date: Saturday, 5. November 2022 at 16:26
To: dev@buildstream.apache.org <de...@buildstream.apache.org>
Subject: Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0
Hey everyone,

> Le mer. 26 oct. 2022 à 15:55, Tristan Van Berkom
> tristan.vanberkom@codethink.co.uk a écrit :
>
> > buildstream-plugins-1.95.3.tar.gz
> > ---------------------------------
> > sha256: 2d33ed4cba762ccc09bbea060e089db08da5ce6150f903a03928da004dcaa387
> > sha512: ee22235884e7dfa54f40bd2baa2df1c26284ce19b4393310cd54dbf60b9789dd075eadacb3189b2002b3254025ed02129fc2e451cadd48ce9ff4da4e8de8a92d
> >
> > BuildStream-1.95.4.dev0.tar.gz
> > ------------------------------
> > sha256: 77f3aafa1268e4128108ac54fd6231cd5b548b0f2b00d84c9c83fc19f7095f60
> > sha512: 7cb335cc837cc70022ac398055e64c691863898daa2a9d0ae89270796b576e2ae692a2583c1a798cc34ba4769f73b92ff98ed26965f2ea2108df2c7ec490bc90
>

-0

I believe https://github.com/apache/buildstream/issues/1787 should be a blocker for this release, as it would otherwise negatively impact the first experience with it.

Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0

Posted by Benjamin Schubert <co...@benschubert.me>.
Hey everyone,

> Le mer. 26 oct. 2022 à 15:55, Tristan Van Berkom
> tristan.vanberkom@codethink.co.uk a écrit :
>
> > buildstream-plugins-1.95.3.tar.gz
> > ---------------------------------
> > sha256: 2d33ed4cba762ccc09bbea060e089db08da5ce6150f903a03928da004dcaa387
> > sha512: ee22235884e7dfa54f40bd2baa2df1c26284ce19b4393310cd54dbf60b9789dd075eadacb3189b2002b3254025ed02129fc2e451cadd48ce9ff4da4e8de8a92d
> >
> > BuildStream-1.95.4.dev0.tar.gz
> > ------------------------------
> > sha256: 77f3aafa1268e4128108ac54fd6231cd5b548b0f2b00d84c9c83fc19f7095f60
> > sha512: 7cb335cc837cc70022ac398055e64c691863898daa2a9d0ae89270796b576e2ae692a2583c1a798cc34ba4769f73b92ff98ed26965f2ea2108df2c7ec490bc90
>

-0

I believe https://github.com/apache/buildstream/issues/1787 should be a blocker for this release, as it would otherwise negatively impact the first experience with it.

Re: [VOTE] Release buildstream / plugins 1.95.4 as 2.0

Posted by Abderrahim Kitouni <a....@gmail.com>.
Hi all

Le mer. 26 oct. 2022 à 15:55, Tristan Van Berkom
<tr...@codethink.co.uk> a écrit :
> buildstream-plugins-1.95.3.tar.gz
> ---------------------------------
> sha256: 2d33ed4cba762ccc09bbea060e089db08da5ce6150f903a03928da004dcaa387
> sha512: ee22235884e7dfa54f40bd2baa2df1c26284ce19b4393310cd54dbf60b9789dd075eadacb3189b2002b3254025ed02129fc2e451cadd48ce9ff4da4e8de8a92d
>
>
> BuildStream-1.95.4.dev0.tar.gz
> ------------------------------
> sha256: 77f3aafa1268e4128108ac54fd6231cd5b548b0f2b00d84c9c83fc19f7095f60
> sha512: 7cb335cc837cc70022ac398055e64c691863898daa2a9d0ae89270796b576e2ae692a2583c1a798cc34ba4769f73b92ff98ed26965f2ea2108df2c7ec490bc90

+1 Looks good to me.