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

Hi,
I’m writing because I see an evident need of making a few commons sense habits
a rule in time in which whatever is not written gets games around with little or no consideration.

In the GeoServer history something deemed worthy of a GSIP has been going though the following steps:

  • Open discussion against a written proposal and vote
  • Actual code development (could start earlier, but at the risk of the one proposing the GSIP)
  • Pull request and review (because accepting a plan does not equate accepting its implementation)
    The first step has some wait time to allow the PSC to review the proposal, it’s now set to 1 week.
    The last one, never had a max time to be performed, but people knew and respected the key developers
    in the area and allowed a bit of time for changes to be reviewed (the longer, the bigger the pull request
    was, with some consideration about avoiding too large pull requests at all costs, given review is not paid).

Unfortunately it seems that common sense and consideration have left this community, replaced by haste and
literal rule application, so I’m proposing to add a time limit for review of pull requests associated to
GSIPs, before automatic merge is done, in order to have some clarity and shared understanding.

Mimicking the proposal discussion, I’d say 1 week time, plus an extra week in case someone
from the PSC needs extra time and explicitly asks for it.

If there are no substantial objections I’m going to write down the proposal in the next few days.

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.


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.

Kind regards,
Ben.

On 10/08/16 07:08, Andrea Aime wrote:

Hi,
I'm writing because I see an evident need of making a few commons sense
habits
a rule in time in which whatever is not written gets games around with
little or no consideration.

In the GeoServer history something deemed worthy of a GSIP has been going
though the following steps:

   - Open discussion against a written proposal and vote
   - Actual code development (could start earlier, but at the risk of the
   one proposing the GSIP)
   - Pull request and review (because accepting a plan does not equate
   accepting its implementation)

The first step has some wait time to allow the PSC to review the proposal,
it's now set to 1 week.
The last one, never had a max time to be performed, but people knew and
respected the key developers
in the area and allowed a bit of time for changes to be reviewed (the
longer, the bigger the pull request
was, with some consideration about avoiding too large pull requests at all
costs, given review is not paid).

Unfortunately it seems that common sense and consideration have left this
community, replaced by haste and
literal rule application, so I'm proposing to add a time limit for review
of pull requests associated to
GSIPs, before automatic merge is done, in order to have some clarity and
shared understanding.

Mimicking the proposal discussion, I'd say 1 week time, plus an extra week
in case someone
from the PSC needs extra time and explicitly asks for it.

If there are no substantial objections I'm going to write down the proposal
in the next few days.

Cheers
Andrea

------------------------------------------------------------------------------
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

--
Ben Caradoc-Davies <ben@anonymised.com>
Director
Transient Software Limited <http://transient.nz/&gt;
New Zealand

On Tue, Aug 9, 2016 at 10:48 PM, Ben Caradoc-Davies <ben@anonymised.com>
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,
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.

-------------------------------------------------------

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
<andrea.aime@anonymised.com> wrote:

On Tue, Aug 9, 2016 at 10:48 PM, Ben Caradoc-Davies <ben@anonymised.com>
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, 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

On Wed, Aug 10, 2016 at 7:50 PM, Simone Giannecchini <
simone.giannecchini@anonymised.com> 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.

-------------------------------------------------------

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.

···

On 2016-08-10 01:20 PM, Andrea Aime wrote:

On Wed, Aug 10, 2016 at 7:50 PM, Simone Giannecchini <simone.giannecchini@anonymised.com> 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](http://sdm.link/zohodev2dev)
_______________________________________________
Geoserver-devel mailing list
[Geoserver-devel@lists.sourceforge.net](mailto:Geoserver-devel@anonymised.comsourceforge.net)
[https://lists.sourceforge.net/lists/listinfo/geoserver-devel](https://lists.sourceforge.net/lists/listinfo/geoserver-devel)

-- 
Kevin Michael Smith
[<smithkm@anonymised.com>](mailto:smithkm@anonymised.com)

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 <smithkm@anonymised.com> wrote:

On 2016-08-10 01:20 PM, Andrea Aime wrote:

On Wed, Aug 10, 2016 at 7:50 PM, Simone Giannecchini
<simone.giannecchini@anonymised.com> 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 reports. http://sdm.link/zohodev2dev

_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

--
Kevin Michael Smith
<smithkm@anonymised.com>

------------------------------------------------------------------------------
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

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 <ben@anonymised.com>
Director
Transient Software Limited <http://transient.nz/&gt;
New Zealand