Re: [Geoserver-devel] Pre-GSIP discussion: clarification on GSIP pull request handling

[Ben Caradoc-Davies]

I am comfortable with PSC members requesting an additional week for 
review, especially for large and/or hairy pull requests.

I think that inclusion and transparency are an important part of 
successful open source projects, and that having reviews by 
non-coworkers promotes this.

Kind regards,

-- 
Ben Caradoc-Davies 
Director
Transient Software Limited 
New Zealand

--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. http://sdm.link/zohodev2dev
___
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Re: [Geoserver-devel] Pre-GSIP discussion: clarification on GSIP pull request handling

[Simone Giannecchini]

Ciao a tutti,
I'd say Andrea, go ahead and draft something up that we can discuss upon.

My interest is to balance quality with the risk of getting important
work delayed or stopped for too long.
But let's be honest, if someone is in a hurry to merge something
"because a client needs it" or some other reasons like "because I need
it" this is simply bad planning. I always tell clients we cannot
guarantee if and when something will get pushed to GeoServer or
GeoNetwork or GeoTools (projects with a decent community) because
there is community discussion and interaction involved.
Let me throw the first stone here and I'll throw it to myself. In the
past I have pushed some contributions too quickly while I should have
fought with the interested parties a bit more to improve quality and
have more reviews.
On the other hand, in ancient times work I had done have been kept in
frozen state for months because a core committer wanted to review and
never had the time. IMHO, this should be avoided, we need a clear
timeline that states when actions should happen that I can refer to
when I do my planning.

So, I'd happy to see some wording that we can discuss upon and refine as we go.



Regards,
Simone Giannecchini
==
GeoServer Professional Services from the experts!
Visit http://goo.gl/it488V for more information.
==
Ing. Simone Giannecchini
@simogeo
Founder/Director

GeoSolutions S.A.S.
Via di Montramito 3/A
55054  Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob:   +39  333 8128928

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

---
AVVERTENZE AI SENSI DEL D.Lgs. 196/2003
Le informazioni contenute in questo messaggio di posta elettronica e/o
nel/i file/s allegato/i sono da considerarsi strettamente riservate.
Il loro utilizzo è consentito esclusivamente al destinatario del
messaggio, per le finalità indicate nel messaggio stesso. Qualora
riceviate questo messaggio senza esserne il destinatario, Vi preghiamo
cortesemente di darcene notizia via e-mail e di procedere alla
distruzione del messaggio stesso, cancellandolo dal Vostro sistema.
Conservare il messaggio stesso, divulgarlo anche in parte,
distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per finalità
diverse, costituisce comportamento contrario ai principi dettati dal
D.Lgs. 196/2003.

The information in this message and/or attachments, is intended solely
for the attention and use of the named addressee(s) and may be
confidential or proprietary in nature or covered by the provisions of
privacy act (Legislative Decree June, 30 2003, no.196 - Italy's New
Data Protection Code).Any use not in accord with its purpose, any
disclosure, reproduction, copying, distribution, or either
dissemination, either whole or partial, is strictly forbidden except
previous formal approval of the named addressee(s). If you are not the
intended recipient, please contact immediately the sender by
telephone, fax or e-mail and delete the information in this message
that has been received in error. The sender does not give any warranty
or accept liability as the content, accuracy or completeness of sent
messages and accepts no responsibility  for changes made after they
were sent or for other risks which arise as a result of e-mail
transmission, viruses, etc.


On Thu, Aug 11, 2016 at 9:22 AM, Kevin Smith  wrote:
> On 2016-08-10 01:20 PM, Andrea Aime wrote:
>
> On Wed, Aug 10, 2016 at 7:50 PM, Simone Giannecchini
>  wrote:
>>
>> -1- we should account for reviews in the GSIP process, although we
>> don't want that for each individual fix but for larger, new
>> functionalities, yes we should.
>> -2- accounting for reviews should not lead to delays for who is
>> proposing the change. So there should be a fixed windows for reviews
>
>
> While I agree, I also believe that no GSIP should go in without some for of
> review.
> While the interested parties can earmark a review, we should not allow some
> strategically (or luckily) placed
> GSIP pull request to just go in without any form of scrutiny.
> A second pair of eyes, even if it's just a serious coworker ones, should be
> applied
>
> Yes, I think that's what we all meant.  There is still the usual procedure
> for merging a pull request in that a committer other than the author should
> review it, this would be an additional requirement that anyone who
> registered a desire to review during discussion of the GSIP must be given a
> week to do so regardless of any other review, although the requested review
> can count as the standard review.
>>
>>
>> So I am fine with something along what Ben proposes:
>>
>> - I can say I'd like to review
>> - I have to do it within 1W at most, that time passed the proposer can
>> hit the merge button (or someone con do it for him)
>
>
> I can live with that, but with a few amendments:
> - For a particularly large request the people volunteering for a 

Re: [Geoserver-devel] Pre-GSIP discussion: clarification on GSIP pull request handling

[Kevin Smith]

On 2016-08-10 01:20 PM, Andrea Aime wrote:
> On Wed, Aug 10, 2016 at 7:50 PM, Simone Giannecchini
>  > wrote:
>
> -1- we should account for reviews in the GSIP process, although we
> don't want that for each individual fix but for larger, new
> functionalities, yes we should.
> -2- accounting for reviews should not lead to delays for who is
> proposing the change. So there should be a fixed windows for reviews
>
>
> While I agree, I also believe that no GSIP should go in without some
> for of review. 
> While the interested parties can earmark a review, we should not allow
> some strategically (or luckily) placed
> GSIP pull request to just go in without any form of scrutiny.
> A second pair of eyes, even if it's just a serious coworker ones,
> should be applied
>
Yes, I think that's what we all meant.  There is still the usual
procedure for merging a pull request in that a committer other than the
author should review it, this would be an additional requirement that
anyone who registered a desire to review during discussion of the GSIP
must be given a week to do so regardless of any other review, although
the requested review can count as the standard review.
>
>
> So I am fine with something along what Ben proposes:
>
> - I can say I'd like to review
> - I have to do it within 1W at most, that time passed the proposer can
> hit the merge button (or someone con do it for him)
>
>
> I can live with that, but with a few amendments:
> - For a particularly large request the people volunteering for a
> review should
>   be allowed to request more time (e.g. think of the resource switch,
> so large that it broke dead the github diff page)
> - No GSIP related pull request ever gets in without someone reviewing
> it and the proposer answering the feedback either by providing
> explanations, or fixing/completing the code. 
> - The pull request does not go in until all feedback is addressed, for
> disagreements on what should be done the question is brought to the
> PSC for discussion
> - A GSIP is normally committed in one shot, with all the bits required
> (tests, docs). The proposer can ask the PSC for leniency in special
> occasions, the PSC will decide if it's ok to allow the
> split/incremental application
>
> Cheers
> Andrea
>
>
> -- 
> ==
> GeoServer Professional Services from the experts! Visit
> http://goo.gl/it488V for more information.
> ==
>
> Ing. Andrea Aime 
> @geowolf
> Technical Lead
>
> GeoSolutions S.A.S.
> Via di Montramito 3/A
> 55054  Massarosa (LU)
> phone: +39 0584 962313
> fax: +39 0584 1660272
> mob: +39  339 8844549
>
> http://www.geo-solutions.it
> http://twitter.com/geosolutions_it
>
> *AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*
>
> Le informazioni contenute in questo messaggio di posta elettronica e/o
> nel/i file/s allegato/i sono da considerarsi strettamente riservate.
> Il loro utilizzo è consentito esclusivamente al destinatario del
> messaggio, per le finalità indicate nel messaggio stesso. Qualora
> riceviate questo messaggio senza esserne il destinatario, Vi preghiamo
> cortesemente di darcene notizia via e-mail e di procedere alla
> distruzione del messaggio stesso, cancellandolo dal Vostro sistema.
> Conservare il messaggio stesso, divulgarlo anche in parte,
> distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per finalità
> diverse, costituisce comportamento contrario ai principi dettati dal
> D.Lgs. 196/2003.
>
>  
>
> The information in this message and/or attachments, is intended solely
> for the attention and use of the named addressee(s) and may be
> confidential or proprietary in nature or covered by the provisions of
> privacy act (Legislative Decree June, 30 2003, no.196 - Italy's New
> Data Protection Code).Any use not in accord with its purpose, any
> disclosure, reproduction, copying, distribution, or either
> dissemination, either whole or partial, is strictly forbidden except
> previous formal approval of the named addressee(s). If you are not the
> intended recipient, please contact immediately the sender by
> telephone, fax or e-mail and delete the information in this message
> that has been received in error. The sender does not give any warranty
> or accept liability as the content, accuracy or completeness of sent
> messages and accepts no responsibility  for changes made after they
> were sent or for other risks which arise as a result of e-mail
> transmission, viruses, etc.
>
>
> ---
>
>
> --
> What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
> patterns at an interface-level. Reveals which users, apps, and protocols are 
> consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
> J-Flow, sFlow and other flows. Make informed decisions using capacity 
> planning 

Re: [Geoserver-devel] Pre-GSIP discussion: clarification on GSIP pull request handling

[Andrea Aime]

On Wed, Aug 10, 2016 at 7:50 PM, Simone Giannecchini <
simone.giannecch...@geo-solutions.it> wrote:

> -1- we should account for reviews in the GSIP process, although we
> don't want that for each individual fix but for larger, new
> functionalities, yes we should.
> -2- accounting for reviews should not lead to delays for who is
> proposing the change. So there should be a fixed windows for reviews
>

While I agree, I also believe that no GSIP should go in without some for of
review.
While the interested parties can earmark a review, we should not allow some
strategically (or luckily) placed
GSIP pull request to just go in without any form of scrutiny.
A second pair of eyes, even if it's just a serious coworker ones, should be
applied


> So I am fine with something along what Ben proposes:
>
> - I can say I'd like to review
> - I have to do it within 1W at most, that time passed the proposer can
> hit the merge button (or someone con do it for him)
>

I can live with that, but with a few amendments:
- For a particularly large request the people volunteering for a review
should
  be allowed to request more time (e.g. think of the resource switch, so
large that it broke dead the github diff page)
- No GSIP related pull request ever gets in without someone reviewing it
and the proposer answering the feedback either by providing explanations,
or fixing/completing the code.
- The pull request does not go in until all feedback is addressed, for
disagreements on what should be done the question is brought to the PSC for
discussion
- A GSIP is normally committed in one shot, with all the bits required
(tests, docs). The proposer can ask the PSC for leniency in special
occasions, the PSC will decide if it's ok to allow the split/incremental
application

Cheers
Andrea


-- 
==
GeoServer Professional Services from the experts! Visit
http://goo.gl/it488V for more information.
==

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via di Montramito 3/A
55054  Massarosa (LU)
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39  339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

*AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*

Le informazioni contenute in questo messaggio di posta elettronica e/o
nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il
loro utilizzo è consentito esclusivamente al destinatario del messaggio,
per le finalità indicate nel messaggio stesso. Qualora riceviate questo
messaggio senza esserne il destinatario, Vi preghiamo cortesemente di
darcene notizia via e-mail e di procedere alla distruzione del messaggio
stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso,
divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od
utilizzarlo per finalità diverse, costituisce comportamento contrario ai
principi dettati dal D.Lgs. 196/2003.



The information in this message and/or attachments, is intended solely for
the attention and use of the named addressee(s) and may be confidential or
proprietary in nature or covered by the provisions of privacy act
(Legislative Decree June, 30 2003, no.196 - Italy's New Data Protection
Code).Any use not in accord with its purpose, any disclosure, reproduction,
copying, distribution, or either dissemination, either whole or partial, is
strictly forbidden except previous formal approval of the named
addressee(s). If you are not the intended recipient, please contact
immediately the sender by telephone, fax or e-mail and delete the
information in this message that has been received in error. The sender
does not give any warranty or accept liability as the content, accuracy or
completeness of sent messages and accepts no responsibility  for changes
made after they were sent or for other risks which arise as a result of
e-mail transmission, viruses, etc.

---
--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. http://sdm.link/zohodev2dev___
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Re: [Geoserver-devel] Pre-GSIP discussion: clarification on GSIP pull request handling

[Simone Giannecchini]

Hi All,
I discussed this a little with Andrea and I believe that, and here is
my feedback.

Baseline is, we should strive to keep quality high. When I talk to end
users in the real world, none of them is happy about the release
early, release often thing they are happier "this works without
issues" thing.

So, IMHO:

-1- we should account for reviews in the GSIP process, although we
don't want that for each individual fix but for larger, new
functionalities, yes we should.
-2- accounting for reviews should not lead to delays for who is
proposing the change. So there should be a fixed windows for reviews

So I am fine with something along what Ben proposes:

- I can say I'd like to review
- I have to do it within 1W at most, that time passed the proposer can
hit the merge button (or someone con do it for him)

Feedback?

Regards,
Simone Giannecchini
==
GeoServer Professional Services from the experts!
Visit http://goo.gl/it488V for more information.
==
Ing. Simone Giannecchini
@simogeo
Founder/Director

GeoSolutions S.A.S.
Via di Montramito 3/A
55054  Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob:   +39  333 8128928

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

---
AVVERTENZE AI SENSI DEL D.Lgs. 196/2003
Le informazioni contenute in questo messaggio di posta elettronica e/o
nel/i file/s allegato/i sono da considerarsi strettamente riservate.
Il loro utilizzo è consentito esclusivamente al destinatario del
messaggio, per le finalità indicate nel messaggio stesso. Qualora
riceviate questo messaggio senza esserne il destinatario, Vi preghiamo
cortesemente di darcene notizia via e-mail e di procedere alla
distruzione del messaggio stesso, cancellandolo dal Vostro sistema.
Conservare il messaggio stesso, divulgarlo anche in parte,
distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per finalità
diverse, costituisce comportamento contrario ai principi dettati dal
D.Lgs. 196/2003.

The information in this message and/or attachments, is intended solely
for the attention and use of the named addressee(s) and may be
confidential or proprietary in nature or covered by the provisions of
privacy act (Legislative Decree June, 30 2003, no.196 - Italy's New
Data Protection Code).Any use not in accord with its purpose, any
disclosure, reproduction, copying, distribution, or either
dissemination, either whole or partial, is strictly forbidden except
previous formal approval of the named addressee(s). If you are not the
intended recipient, please contact immediately the sender by
telephone, fax or e-mail and delete the information in this message
that has been received in error. The sender does not give any warranty
or accept liability as the content, accuracy or completeness of sent
messages and accepts no responsibility  for changes made after they
were sent or for other risks which arise as a result of e-mail
transmission, viruses, etc.


On Wed, Aug 10, 2016 at 3:31 AM, Andrea Aime
 wrote:
> On Tue, Aug 9, 2016 at 10:48 PM, Ben Caradoc-Davies 
> wrote:
>>
>> Andrea,
>>
>> we raised this problem in the committee meeting. Rather than a mandatory
>> delay, we could add an "interested parties" section to each GSIP so that
>> those wanting to do so could advertise their desire to review pull requests
>> for the GSIP. We could add a rule to the GSIP procedure that all subscribers
>> listed in the "interested parties" section must be given a week (or two if
>> requested?) to review pull requests, even after the pull request is
>> accepted. This would allow clearer communication of interest and
>> availability.
>
>
> Hi Ben,
> it's interesting that you name it a "mandatory delay", given my experience
> with pull requests related to GSIPs and new functionality
> in general that sounds more like a mandatory speedup, I hardly see any pull
> request of significance being handled within the week (normally
> because we allow others to have a look at it before merging).
> Indeed, to make a parallel, the week long rules about voting on the
> proposals were meant to avoid long delays, not to force a slowdown.
>
> The particular context here spins it as a delay because we have just
> experience something novel, a GSIP related
> pull request being merged so quickly that "interested parties" did not even
> have time to check their availability, let alone do a review.
>
> Allowing a bit of time to review a GSIP related pull request is important,
> for a number of good reasons:
>
> If the proposal was high level, details that might spun acceptance one way
> or the other can only be seen in code. The PSC should be allowed to check
> what actually happened code wise in this case.
> If the the proposal was detailed and low level, actual implementation could
> have diverged from it in some respect, normally because hitting the actual
> code brings insight not available when writing the proposal. 

Re: [Geoserver-devel] Pre-GSIP discussion: clarification on GSIP pull request handling

[Andrea Aime]

On Tue, Aug 9, 2016 at 10:48 PM, Ben Caradoc-Davies 
wrote:

> Andrea,
>
> we raised this problem in the committee meeting. Rather than a mandatory
> delay, we could add an "interested parties" section to each GSIP so that
> those wanting to do so could advertise their desire to review pull requests
> for the GSIP. We could add a rule to the GSIP procedure that all
> subscribers listed in the "interested parties" section must be given a week
> (or two if requested?) to review pull requests, even after the pull request
> is accepted. This would allow clearer communication of interest and
> availability.
>

Hi Ben,
it's interesting that you name it a "mandatory delay", given my experience
with pull requests related to GSIPs and new functionality
in general that sounds more like a mandatory speedup, I hardly see any pull
request of significance being handled within the week (normally
because we allow others to have a look at it before merging).
Indeed, to make a parallel, the week long rules about voting on the
proposals were meant to avoid long delays, not to force a slowdown.

The particular context here spins it as a delay because we have just
experience something novel, a GSIP related
pull request being merged so quickly that "interested parties" did not even
have time to check their availability, let alone do a review.

Allowing a bit of time to review a GSIP related pull request is important,
for a number of good reasons:

   - If the proposal was high level, details that might spun acceptance one
   way or the other can only be seen in code. The PSC should be allowed to
   check what actually happened code wise in this case.
   - If the the proposal was detailed and low level, actual implementation
   could have diverged from it in some respect, normally because hitting the
   actual code brings insight not available when writing the proposal. While
   it would be too much to go back, change the proposal, and vote again based
   on implementation results, the PSC should be allowed to check the
   divergence is not so significant as to make the proposal invalid.
   - If something is proposal worthy, it's likely touching highly visible
   portions of the user experience, or core code, so it's only natural that we
   should check whether it's affecting backwards compatibility, performance
   and stability.

Generally speaking, it should be natural that if a pull request is related
to GSIP, it should go though more scrutiny, instead here we have some party
claiming that "it was voted on", so it can go in even as fast, if not
faster, than the average pull request. But as I said, if you check the pull
request queue, you'll find that changes of any significant often take more
than a week to be reviewed.
What I'm trying to push for is actually a process that's quicker than our
"tradition", disallowing "flash merges" but also setting up some
fairness/level playing field.

Give the above, having to explicitly specify "interested parties" does not
seem like a good way to go, as the PSC is the "interested party" by
definition of GSIP.

Taking GSIP-149 as an example, the pull request is not small (40 files
changed) and diverges from the GSIP by downgrading to "not implemented nice
to haves" some bits that were official part of the proposal (see
description at https://github.com/geoserver/geoserver/pull/1748 and compare
to  https://github.com/geoserver/geoserver/wiki/GSIP-149) .
Personally I don't think the latter is a problem (but it's just me, others
might have a different opinion), although I'm surprised to see that the CSS
editor pages are still there (the proposal states "the CSS Styles page
would be deleted from the CSS Styling extension") and that no documentation
has been updated (but the style editor works in a significantly different
way now, so doc updates are actually mandatory).
Tomorrow (well, actually later today, it's Wednesday already here) I'll try
to check the actual code changes, but now that the pull request has been
merged, will there be any incentive to address the review?

Cheers
Andrea





-- 
==
GeoServer Professional Services from the experts! Visit
http://goo.gl/it488V for more information.
==

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via di Montramito 3/A
55054  Massarosa (LU)
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39  339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

*AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*

Le informazioni contenute in questo messaggio di posta elettronica e/o
nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il
loro utilizzo è consentito esclusivamente al destinatario del messaggio,
per le finalità indicate nel messaggio stesso. Qualora riceviate questo
messaggio senza esserne il destinatario, Vi preghiamo cortesemente di
darcene notizia via e-mail e di procedere alla distruzione del messaggio
stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso,