commitfest 2016-11 status summary

On Tue, Nov 1, 2016 at 3:48 PM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:

There are plenty of patches that are in "ready for committer" state,
committers please have a look at those patches and give some conclusion
on them.

Yes, we really need some more committer attention on a lot of these
patches. I've been trying to do what I can to whittle that list down,
but it's long and contains many complex patches, as well as quite a
few patches in areas that I don't really understand. For example, I
have no idea whether these things are good ideas, because I don't know
Windows:

https://commitfest.postgresql.org/11/604/ - pgwin32_is_service not
checking if SECURITY_SERVICE_SID is disabled
https://commitfest.postgresql.org/11/620/ - Updating Windows
environment variables

Any chance that some Windows-savvy committer can take a look at those?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Nov 4, 2016 at 12:36 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Tue, Nov 1, 2016 at 3:48 PM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:
https://commitfest.postgresql.org/11/604/ - pgwin32_is_service not
checking if SECURITY_SERVICE_SID is disabled

This is quite a particular fix in a particular context. This is as
well waiting some input from the reporter to be sure that the patch
proposed is fixing what his case. The patch should fix it, but that's
a matter to be sure now.

https://commitfest.postgresql.org/11/620/ - Updating Windows
environment variables

This one is not that particular. Any committer input is welcome. All
the patches proposed should be HEAD-only considering the lack of
complaints across the years.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi All,

The commitfest status summary after one week of progress:

Needs review: 78
Waiting on author: 16
Ready for Commiter: 19
Commited: 28
Moved to next CF: 0
Rejected: 3
Returned with feedback: 3
TOTAL: 147

Overall progress of completion - 23%.
week-1 progress of completion - 9%.

There are many patches that are in "ready for committer" state, committers
are working hard to move the patches, but there are many patches that are
getting added into "ready for committer" state.

There are some patches on "waiting on author" with no updates from author
for a long time, please take some action within 1-3 days, otherwise the
patch
status can be marked as "returned with feedback" for the smoother operation
of the commitfest.

There are patches with the reviewer assigned, but no review is shared yet.
Please try to share your review related to the patch in 1-3 days. In case
if the
reviewer is busy with some other work, you can remove yourself as a
reviewer,
so that at least someone can review that patch. I will try to ask the
reviewer
in the corresponding thread if the patch didn't receive any review after 3
days.
This is not to blame some one, just to ensure the smooth progress of
commitfest.

There are still some patches with no reviewer assigned. Please consider
reviewing some patches. The review can be a code, test and etc, whichever is
comfortable to you. We need your help.

There are some authors who submitted patches to the commitfest but not
reviewing
any patch as a reviewer. Please try to review a patch that is suitable to
you. We need
your help.

Thanks everyone for smoother operation of commitfest.

Regards,
Hari Babu
Fujitsu Australia

On Sun, Nov 6, 2016 at 3:33 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Fri, Nov 4, 2016 at 12:36 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Tue, Nov 1, 2016 at 3:48 PM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:
https://commitfest.postgresql.org/11/604/ - pgwin32_is_service not
checking if SECURITY_SERVICE_SID is disabled

This is quite a particular fix in a particular context. This is as
well waiting some input from the reporter to be sure that the patch
proposed is fixing what his case. The patch should fix it, but that's
a matter to be sure now.

Well, then it should be marked "Waiting on Author" or "Needs Review",
not "Ready for Committer".

Or maybe just "Returned with Feedback", if the reporter hasn't poked
his head up for a few weeks.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Nov 9, 2016 at 10:53 AM, Robert Haas <robertmhaas@gmail.com> wrote:

Well, then it should be marked "Waiting on Author" or "Needs Review",
not "Ready for Committer".

Or maybe just "Returned with Feedback", if the reporter hasn't poked
his head up for a few weeks.

Since yesterday with the help of Tsunakawa-san, we found a simple way
to test things by patching pg_ctl.c so "Ready for Committer"
definitely applies now. This has allowed us to draft patches that fix
the original issue as well as another issue we bumped on the way:
Postgres being started by the Local System user does not ensure that
it is a service. The instance may have been started by another service
expecting Postgres to report to stderr.

There is a refactoring patch that simplifies the code of
win32security.c for HEAD, as well as a simple back-patchable fix for
other branches.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi All,

The commitfest status summary after one week of progress:

Needs review: 76
Waiting on author: 16
Ready for Commiter: 16
Commited: 32
Moved to next CF: 0
Rejected: 4
Returned with feedback: 3
TOTAL: 147

Overall progress of completion - 26%.
week-1 progress of completion - 9%.
week-2 progress of completion - 3%

The progress in this week is slow compared to earlier week.

There are patches with the reviewer assigned, but no review is shared yet.
I will try to ask the reviewer in the corresponding thread for a review.

There are still some patches with no reviewer assigned. Please consider
reviewing some patches. The review can be a code, test and etc, whichever is
comfortable to you. We need your help.

Regards,
Hari Babu
Fujitsu Australia

Hi All,

The commitfest status summary after three weeks of progress:

Needs review: 59
Waiting on author: 28
Ready for Commiter: 16
Commited: 35
Moved to next CF: 0
Rejected: 5
Returned with feedback: 4
TOTAL: 147

Overall progress of completion - 29%
week-1 progress of completion - 9%
week-2 progress of completion - 3%
week-3 progress of completion - 3%

Last two weeks the commitfest progress is slow compared to first week.

There are many patches in "ready for committer" state, committer's please
have a check on those patches and provide your update.

I sent reminder mails to the reviewers who haven't responded to the patches
in those corresponding threads. Please share your review, this will help us
in smoother operation of commitfest.

There are still some patches with no reviewer assigned. Please consider
reviewing some patches. The review can be a code, test and etc, whichever is
comfortable to you. We need your help.

I updated the status of some patches as "waiting on author" to reflect it's
actual state based on my understanding of recent communications on those
threads. If anyone feels the status is wrong, please update it accordingly.

Regards,
Hari Babu
Fujitsu Australia

Hi All,

The commitfest status summary after four weeks of progress:

Needs review: 48
Waiting on author: 25
Ready for Commiter: 12
Commited: 36
Moved to next CF: 15
Rejected: 5
Returned with feedback: 6
TOTAL: 147

Overall progress of completion - 42%
week-1 progress of completion - 9%
week-2 progress of completion - 3%
week-3 progress of completion - 3%
week-4 progress of completion - 13%

Because of many patches that are moved to next CF, there is good progress
in this week.

As It is almost end of the commitfest, there are many patches in
"ready for committer" state, committer's please have a check on those
patches and provide your update.

There are many patches on "waiting on author" state, authors, please respond
to those patches either by providing updated patch or moving the patch into
next commitfest. In case if author doesn't respond, those will be moved into
"returned with feedback" state.

Regards,
Hari Babu
Fujitsu Australia

Hi All,

The commitfest status summary at the end of commitfest.

Needs review: 0
Waiting on author: 0
Ready for Commiter: 0
Commited: 41
Moved to next CF: 79
Rejected: 7
Returned with feedback: 20
TOTAL: 147

Overall progress of completion - 46% (doesn't include "moved to next CF")

Micheal, I need your help in closing the commitfest.

I closed the commitfest using the following assumptions.

Moved to next CF with needs review.
1. patch doesn't receive any full review in the commitfest
2. Patch received feedback at the end of commitfest.

Moved to next CF with waiting on author:
1. Patch doesn't apply to HEAD, but didn't receive any feedback.

Returned with feedback:
1. Patch received feedback, but author hasn't responded yet.
2. Author is expected to share an updated patch.

Rejected:
1. Any -1 from committer to the approach of the patch

May be these assumptions needs to be updated, as this is the first
time as CFM for me.

As I observed many patches that are keep on moving to next CF from previous
patches, is there any way in commitfest that can highlight those patches,
so that
those patches gets the review first than the patches that are came late to
the
commitfest.

I definitely may missed judging the current state of the patch. Please feel
free to
update the actual status.

Thanks everyone.

Regards,
Hari Babu
Fujitsu Australia

On Mon, Dec 5, 2016 at 2:50 PM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:

Micheal, I need your help in closing the commitfest.

And done. Thanks for doing this work! I was ready to use my CF vacuum
cleaner as need be, but that does not seem to be needed.

I closed the commitfest using the following assumptions.

Moved to next CF with needs review.
1. patch doesn't receive any full review in the commitfest
2. Patch received feedback at the end of commitfest.

Moved to next CF with waiting on author:
1. Patch doesn't apply to HEAD, but didn't receive any feedback.

Returned with feedback:
1. Patch received feedback, but author hasn't responded yet.
2. Author is expected to share an updated patch.

Rejected:
1. Any -1 from committer to the approach of the patch

All that sounds fair to me.

May be these assumptions needs to be updated, as this is the first
time as CFM for me.

I definitely may missed judging the current state of the patch. Please feel
free to update the actual status.

That's definitely up to the authors and reviewers to double-check what
has been done and correct anything they think is not. And far as I can
see you have done an admirable work in keeping it on tracks, so huge
+1 from me.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Dec 5, 2016 at 11:56 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Mon, Dec 5, 2016 at 2:50 PM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:

I definitely may missed judging the current state of the patch. Please feel
free to update the actual status.

That's definitely up to the authors and reviewers to double-check what
has been done and correct anything they think is not. And far as I can
see you have done an admirable work in keeping it on tracks, so huge
+1 from me.

I also think Hari has done a good job in wrapping up this commitfest.
However, we might want to consider sending one consolidated mail for
the patches "moved to next CF" or "Returned with Feedback" or
"Rejected" rather than sending independent mails. That might save
some traffic on the list.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

* Amit Kapila (amit.kapila16@gmail.com) wrote:

On Mon, Dec 5, 2016 at 11:56 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Mon, Dec 5, 2016 at 2:50 PM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:

I definitely may missed judging the current state of the patch. Please feel
free to update the actual status.

That's definitely up to the authors and reviewers to double-check what
has been done and correct anything they think is not. And far as I can
see you have done an admirable work in keeping it on tracks, so huge
+1 from me.

I also think Hari has done a good job in wrapping up this commitfest.
However, we might want to consider sending one consolidated mail for
the patches "moved to next CF" or "Returned with Feedback" or
"Rejected" rather than sending independent mails. That might save
some traffic on the list.

Agreed. The status change is noted in the CF app, it doesn't need to be
included in the thread. Further, having the info about what happened to
each patch in one consolidated email would be a lot easier for people to
review than an email on every thread.

Thanks!

Stephen

On Tue, Dec 6, 2016 at 12:59 AM, Stephen Frost <sfrost@snowman.net> wrote:

* Amit Kapila (amit.kapila16@gmail.com) wrote:

On Mon, Dec 5, 2016 at 11:56 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Mon, Dec 5, 2016 at 2:50 PM, Haribabu Kommi <

kommi.haribabu@gmail.com> wrote:

I definitely may missed judging the current state of the patch.

Please feel

free to update the actual status.

That's definitely up to the authors and reviewers to double-check what
has been done and correct anything they think is not. And far as I can
see you have done an admirable work in keeping it on tracks, so huge
+1 from me.

I also think Hari has done a good job in wrapping up this commitfest.
However, we might want to consider sending one consolidated mail for
the patches "moved to next CF" or "Returned with Feedback" or
"Rejected" rather than sending independent mails. That might save
some traffic on the list.

Agreed. The status change is noted in the CF app, it doesn't need to be
included in the thread. Further, having the info about what happened to
each patch in one consolidated email would be a lot easier for people to
review than an email on every thread.

Thanks for your opinions. I will do the same in future.

Regards,
Hari Babu
Fujitsu Australia

On Mon, Dec 5, 2016 at 12:50 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

Moved to next CF with needs review.
1. patch doesn't receive any full review in the commitfest
2. Patch received feedback at the end of commitfest.

The number of patches that got this treatment seems quite large. If
the people who are submitting patches are also reviewing other patches
of comparable difficulty, this really shouldn't be happening.
Something isn't working well, here.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 12/5/16 3:27 PM, Robert Haas wrote:

On Mon, Dec 5, 2016 at 12:50 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

Moved to next CF with needs review.
1. patch doesn't receive any full review in the commitfest
2. Patch received feedback at the end of commitfest.

The number of patches that got this treatment seems quite large. If
the people who are submitting patches are also reviewing other patches
of comparable difficulty, this really shouldn't be happening.
Something isn't working well, here.

Yeah. :( (and I'm embarrassed to admit I'm part of the problem :()

I think it would be good to have a different status for patches that got
bumped to the next CF due to insufficient review. It's already unfair to
authors, but at least if there was a separate status those patches could
be prioritized next time around. It would also allow tracking how big a
problem that is from one CF to the next.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers