New CF app deployment

Hi!

Last I said something about the new CF app I said I was planning to deploy
it over the holidays, and that clearly did not happen.

But I've been talking to Michael, as the current cf-chief, and doing some
further testing with it, and we think now is a good time to go for it :) As
the plan is to close out the current CF just a few days from now, we're
going to use that and try to swap it out when traffic is at least at it's
lowest (even if we're well aware it's not going to be zero).

So based on this, we plan to:

In the late evening on Jan 19th (evening European time that is), I will
make the current CF app readonly, and move it to a new url where it will
remain available for the foreseeable future (we have a lot of useful data
in it after all).

Once this is done, Michael (primarily) will start working on syncing up the
information about the latest patches into the new app. Much info is already
synced there, but to make sure all the latest changes are included.

In the morning European time on the 20th, I'll take over and try to finish
up what's left. And sometime during the day when things are properly in
sync, we'll open up the new app for business to all users.

There are surely some bugs remaining in the system, so please have some
oversight with that over the coming days/weeks. I'll try to get onto fixing
them as soon as I can - and some others can look at that as well if it's
something critical at least.

Further status updates to come as we start working on it...

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Tue, Jan 13, 2015 at 2:35 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> Further status updates to come as we start working on it...

Things are looking good so far. All the information has been synced up
between both apps for the current CF and the next one. One the switch
is done, I would recommend to each patch author and reviewer to check
the patches they are registered on, and do the necessary modifications
if we missed something. Error is human.

Note as well that the new CF app has a couple of differences with the
old app regarding the patch status:
- When a patch is marked as "returned with feedback", it is
automatically added to the next CF
- When a patch is marked as "rejected", it is *not* added in the next
CF, but you can still re-add a new entry manually in the next app.
So "rejected" means as well that a patch is marked as such because the
author does not have time/resources to work on it for the next CF(s).
Thanks,
--
Michael

On Mon, Jan 19, 2015 at 3:16 AM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:

> On Tue, Jan 13, 2015 at 2:35 PM, Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> > Further status updates to come as we start working on it...
>
> Things are looking good so far. All the information has been synced up
> between both apps for the current CF and the next one. One the switch
> is done, I would recommend to each patch author and reviewer to check
> the patches they are registered on, and do the necessary modifications
> if we missed something. Error is human.
>
> Note as well that the new CF app has a couple of differences with the
> old app regarding the patch status:
> - When a patch is marked as "returned with feedback", it is
> automatically added to the next CF
> - When a patch is marked as "rejected", it is *not* added in the next
> CF, but you can still re-add a new entry manually in the next app.
> So "rejected" means as well that a patch is marked as such because the
> author does not have time/resources to work on it for the next CF(s).
> Thanks,
>

The new site has been deployed and should now be usable.

The old site is still available in readonly mode at
https://commitfest-old.postgresql.org/.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Mon, Jan 19, 2015 at 3:57 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> The new site has been deployed and should now be usable.
>
> The old site is still available in readonly mode at
> https://commitfest-old.postgresql.org/.

These links, which were originally requested by Tom and which I have
bookmarked, used, and publicized extensively, no longer work:

https://commitfest.postgresql.org/action/commitfest_view/inprogress
https://commitfest.postgresql.org/action/commitfest_view/open

Any chance you could make those work, or at the very least provide
some equivalent?

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

On Mon, Jan 19, 2015 at 10:01 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, Jan 19, 2015 at 3:57 PM, Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> > The new site has been deployed and should now be usable.
> >
> > The old site is still available in readonly mode at
> > https://commitfest-old.postgresql.org/.
>
> These links, which were originally requested by Tom and which I have
> bookmarked, used, and publicized extensively, no longer work:
>
> https://commitfest.postgresql.org/action/commitfest_view/inprogress
> https://commitfest.postgresql.org/action/commitfest_view/open
>
> Any chance you could make those work, or at the very least provide
> some equivalent?
>

Hmm. I missed that request.

Will fix.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Mon, Jan 19, 2015 at 3:57 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> The new site has been deployed and should now be usable.

There are, for some reason, three copies of "Clarify need for memory
barriers in bgworkers" in the in-progress CF. I don't know why that
happened, or how to fix it.

Also, the old app did this:

Status Summary. Needs Review: 34, Waiting on Author: 6, Ready for
Committer: 5, Committed: 16, Returned with Feedback: 12, Rejected: 5.
Total: 78.

...and you could click on the statuses to filter by that status. I
see that this new app has a different way to filter by status, which
is fine, but I though the quick summary at the top was awfully useful,
so you could see how many total patches there were and how much
progress we were making.

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

On Mon, Jan 19, 2015 at 4:05 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Mon, Jan 19, 2015 at 3:57 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>> The new site has been deployed and should now be usable.
>
> There are, for some reason, three copies of "Clarify need for memory
> barriers in bgworkers" in the in-progress CF. I don't know why that
> happened, or how to fix it.

There are also two copies of "ctidscan as an example of custom-scan".

Also, I can't figure out what I'm supposed to do with the "Author" and
"Reviewer" checkboxes. Checking and unchecking them doesn't seem to
do anything.

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

On Mon, Jan 19, 2015 at 10:03 PM, Magnus Hagander <magnus(at)hagander(dot)net>
wrote:

> On Mon, Jan 19, 2015 at 10:01 PM, Robert Haas <robertmhaas(at)gmail(dot)com>
> wrote:
>
>> On Mon, Jan 19, 2015 at 3:57 PM, Magnus Hagander <magnus(at)hagander(dot)net>
>> wrote:
>> > The new site has been deployed and should now be usable.
>> >
>> > The old site is still available in readonly mode at
>> > https://commitfest-old.postgresql.org/.
>>
>> These links, which were originally requested by Tom and which I have
>> bookmarked, used, and publicized extensively, no longer work:
>>
>> https://commitfest.postgresql.org/action/commitfest_view/inprogress
>> https://commitfest.postgresql.org/action/commitfest_view/open
>>
>> Any chance you could make those work, or at the very least provide
>> some equivalent?
>>
>
> Hmm. I missed that request.
>
> Will fix.
>

Fixed. Those were easy :)

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Mon, Jan 19, 2015 at 10:05 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, Jan 19, 2015 at 3:57 PM, Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> > The new site has been deployed and should now be usable.
>
> There are, for some reason, three copies of "Clarify need for memory
> barriers in bgworkers" in the in-progress CF. I don't know why that
> happened, or how to fix it.
>
> Also, the old app did this:
>
> Status Summary. Needs Review: 34, Waiting on Author: 6, Ready for
> Committer: 5, Committed: 16, Returned with Feedback: 12, Rejected: 5.
> Total: 78.
>
> ...and you could click on the statuses to filter by that status. I
> see that this new app has a different way to filter by status, which
> is fine, but I though the quick summary at the top was awfully useful,
> so you could see how many total patches there were and how much
> progress we were making.
>
>
I'll add that to my TODO - but it won't be fixed tonight.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Mon, Jan 19, 2015 at 10:10 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, Jan 19, 2015 at 4:05 PM, Robert Haas <robertmhaas(at)gmail(dot)com>
> wrote:
> > On Mon, Jan 19, 2015 at 3:57 PM, Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> >> The new site has been deployed and should now be usable.
> >
> > There are, for some reason, three copies of "Clarify need for memory
> > barriers in bgworkers" in the in-progress CF. I don't know why that
> > happened, or how to fix it.
>
> There are also two copies of "ctidscan as an example of custom-scan".
>

Yeah, Michael pointed out he was unable to fix that one. I think I've fixed
the permissions errors required for that, so I'll wait for him to try again
to confirm that I managed to fix it.

Also, I can't figure out what I'm supposed to do with the "Author" and
> "Reviewer" checkboxes. Checking and unchecking them doesn't seem to
> do anything.
>
>
That's part of the "mass email to reviewers/authors" that's available only
to cf managers (which you are flagged as in the system). There's a button
to actually send the mail at the bottom (pick "selected" as receipient).

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Tue, Jan 20, 2015 at 6:54 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Mon, Jan 19, 2015 at 10:10 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>
>> On Mon, Jan 19, 2015 at 4:05 PM, Robert Haas <robertmhaas(at)gmail(dot)com>
>> wrote:
>> > On Mon, Jan 19, 2015 at 3:57 PM, Magnus Hagander <magnus(at)hagander(dot)net>
>> > wrote:
>> >> The new site has been deployed and should now be usable.
>> >
>> > There are, for some reason, three copies of "Clarify need for memory
>> > barriers in bgworkers" in the in-progress CF. I don't know why that
>> > happened, or how to fix it.
>>
>> There are also two copies of "ctidscan as an example of custom-scan".
>
>
> Yeah, Michael pointed out he was unable to fix that one. I think I've fixed
> the permissions errors required for that, so I'll wait for him to try again
> to confirm that I managed to fix it.
Thanks for fixing the permission problem. I have removed the
duplicated entries, something actually caused by me when adding the
existing patches to the new app.
--
Michael

Hello, the new app's clean looking gets my favor and the
integrated operation will do good for me:)

By the way, I got the following notice when I tried to "Add
comment" on the new app.

"Note!: This form will generate an email to the public
mailinglist pgsql-hackers, with sender set to
horiguchi(dot)kyotaro(at)oss(dot)ntt(dot)co(dot)jp!"

Hmm. The mail address indeed *was* mine but is now obsolete, so
that the email might bounce. But I haven't find how to change it
within the app itself, and the PostgreSQL community account page.

https://www.postgresql.org/account/

Could you tell me how do I change the sender address?

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Hi,

On 2015-01-19 21:57:20 +0100, Magnus Hagander wrote:
> The new site has been deployed and should now be usable.

I think this unfortunately lost the RSS feature? I found that quite
useful to see who changed what recently (it's forwared to an imap
mailbox for me...).

What I'm also missing from the old app is that previously 'reviews'
could explicitly be linked from the app. Now there's a list of emails in
the thread, nice!, but in bigger threads that really doesn't help to
find the actual review.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Tue, Jan 20, 2015 at 10:09 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> Hi,
>
> On 2015-01-19 21:57:20 +0100, Magnus Hagander wrote:
>> The new site has been deployed and should now be usable.
>
> I think this unfortunately lost the RSS feature? I found that quite
> useful to see who changed what recently (it's forwared to an imap
> mailbox for me...).
Yep, added here:
https://commitfest.postgresql.org/3/109/
I linked it with 20140922230255(dot)GD2521(at)awork2(dot)anarazel(dot)de(dot) I also
recall that you and Robert were marked as reviewers, right?
--
Michael

On 2015-01-20 11:05:43 +0900, Michael Paquier wrote:
> On Tue, Jan 20, 2015 at 10:09 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > Hi,
> >
> > On 2015-01-19 21:57:20 +0100, Magnus Hagander wrote:
> >> The new site has been deployed and should now be usable.
> >
> > I think this unfortunately lost the RSS feature? I found that quite
> > useful to see who changed what recently (it's forwared to an imap
> > mailbox for me...).
> Yep, added here:
> https://commitfest.postgresql.org/3/109/
> I linked it with 20140922230255(dot)GD2521(at)awork2(dot)anarazel(dot)de(dot) I also
> recall that you and Robert were marked as reviewers, right?

I think you misunderstood me ;). I was talking about the old CF
application providing a RSS feed of all changes to all entries.

I.e.
https://commitfest-old.postgresql.org/action/commitfest_activity.rss

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Tue, Jan 20, 2015 at 4:31 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2015-01-20 11:05:43 +0900, Michael Paquier wrote:
>> On Tue, Jan 20, 2015 at 10:09 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> > Hi,
>> >
>> > On 2015-01-19 21:57:20 +0100, Magnus Hagander wrote:
>> >> The new site has been deployed and should now be usable.
>> >
>> > I think this unfortunately lost the RSS feature? I found that quite
>> > useful to see who changed what recently (it's forwared to an imap
>> > mailbox for me...).
>> Yep, added here:
>> https://commitfest.postgresql.org/3/109/
>> I linked it with 20140922230255(dot)GD2521(at)awork2(dot)anarazel(dot)de(dot) I also
>> recall that you and Robert were marked as reviewers, right?
>
> I think you misunderstood me ;). I was talking about the old CF
> application providing a RSS feed of all changes to all entries.
>
> I.e.
> https://commitfest-old.postgresql.org/action/commitfest_activity.rss
Oh, I didn't know this one. That's indeed useful.
--
Michael

>>>>> "Kyotaro" == Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> writes:

Kyotaro> Hmm. The mail address indeed *was* mine but is now obsolete,
Kyotaro> so that the email might bounce. But I haven't find how to
Kyotaro> change it within the app itself, and the PostgreSQL community
Kyotaro> account page.

Just being able to change the email address on the community account
isn't enough; I for one am subscribed to the lists with a different
email address than the one associated with my community account (for
spam management reasons). There needs to be a way to have multiple
addresses or to specify which is to be used for the post.

--
Andrew (irc:RhodiumToad)

Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
> On Tue, Jan 20, 2015 at 4:31 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> I think you misunderstood me ;). I was talking about the old CF
>> application providing a RSS feed of all changes to all entries.
>> https://commitfest-old.postgresql.org/action/commitfest_activity.rss

> Oh, I didn't know this one. That's indeed useful.

Personally, I never used the RSS feed as such, but I did often consult the
"activity log" pages, which I think are equivalent:

https://commitfest-old.postgresql.org/action/commitfest_activity?id=25

That feature seems to be gone too :-(

regards, tom lane

On Tue, Jan 20, 2015 at 3:43 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
> > On Tue, Jan 20, 2015 at 4:31 PM, Andres Freund <andres(at)2ndquadrant(dot)com>
> wrote:
> >> I think you misunderstood me ;). I was talking about the old CF
> >> application providing a RSS feed of all changes to all entries.
> >> https://commitfest-old.postgresql.org/action/commitfest_activity.rss
>
> > Oh, I didn't know this one. That's indeed useful.
>
> Personally, I never used the RSS feed as such, but I did often consult the
> "activity log" pages, which I think are equivalent:
>
> https://commitfest-old.postgresql.org/action/commitfest_activity?id=25
>
> That feature seems to be gone too :-(
>

Yeah, that's annoying.

In fact, I'm the one who forced the RSS feed into the old system, and it's
something I use all the time myself. Oops.

Turns out I have those in a feature branch that it appears I forgot to
merge :( And it also no longer merges cleanly.

I've added this to my short-term TODO, and will look at it this evening.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Hi

I cannot to set my name as author for patch:
https://commitfest.postgresql.org/4/112/

Regards

Pavel

2015-01-13 6:35 GMT+01:00 Magnus Hagander <magnus(at)hagander(dot)net>:

> Hi!
>
> Last I said something about the new CF app I said I was planning to deploy
> it over the holidays, and that clearly did not happen.
>
> But I've been talking to Michael, as the current cf-chief, and doing some
> further testing with it, and we think now is a good time to go for it :) As
> the plan is to close out the current CF just a few days from now, we're
> going to use that and try to swap it out when traffic is at least at it's
> lowest (even if we're well aware it's not going to be zero).
>
> So based on this, we plan to:
>
> In the late evening on Jan 19th (evening European time that is), I will
> make the current CF app readonly, and move it to a new url where it will
> remain available for the foreseeable future (we have a lot of useful data
> in it after all).
>
> Once this is done, Michael (primarily) will start working on syncing up
> the information about the latest patches into the new app. Much info is
> already synced there, but to make sure all the latest changes are included.
>
> In the morning European time on the 20th, I'll take over and try to finish
> up what's left. And sometime during the day when things are properly in
> sync, we'll open up the new app for business to all users.
>
> There are surely some bugs remaining in the system, so please have some
> oversight with that over the coming days/weeks. I'll try to get onto fixing
> them as soon as I can - and some others can look at that as well if it's
> something critical at least.
>
> Further status updates to come as we start working on it...
>
> --
> Magnus Hagander
> Me: http://www.hagander.net/
> Work: http://www.redpill-linpro.com/
>

2015-01-20 19:16 GMT+01:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:

> Hi
>
> I cannot to set my name as author for patch:
> https://commitfest.postgresql.org/4/112/
>

It is solved now - I don't understand a autocomplete in first moment

All works well

Regards

Pavel

>
> Regards
>
> Pavel
>
> 2015-01-13 6:35 GMT+01:00 Magnus Hagander <magnus(at)hagander(dot)net>:
>
>> Hi!
>>
>> Last I said something about the new CF app I said I was planning to
>> deploy it over the holidays, and that clearly did not happen.
>>
>> But I've been talking to Michael, as the current cf-chief, and doing some
>> further testing with it, and we think now is a good time to go for it :) As
>> the plan is to close out the current CF just a few days from now, we're
>> going to use that and try to swap it out when traffic is at least at it's
>> lowest (even if we're well aware it's not going to be zero).
>>
>> So based on this, we plan to:
>>
>> In the late evening on Jan 19th (evening European time that is), I will
>> make the current CF app readonly, and move it to a new url where it will
>> remain available for the foreseeable future (we have a lot of useful data
>> in it after all).
>>
>> Once this is done, Michael (primarily) will start working on syncing up
>> the information about the latest patches into the new app. Much info is
>> already synced there, but to make sure all the latest changes are included.
>>
>> In the morning European time on the 20th, I'll take over and try to
>> finish up what's left. And sometime during the day when things are properly
>> in sync, we'll open up the new app for business to all users.
>>
>> There are surely some bugs remaining in the system, so please have some
>> oversight with that over the coming days/weeks. I'll try to get onto fixing
>> them as soon as I can - and some others can look at that as well if it's
>> something critical at least.
>>
>> Further status updates to come as we start working on it...
>>
>> --
>> Magnus Hagander
>> Me: http://www.hagander.net/
>> Work: http://www.redpill-linpro.com/
>>
>
>

On Tue, Jan 20, 2015 at 3:43 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
> > On Tue, Jan 20, 2015 at 4:31 PM, Andres Freund <andres(at)2ndquadrant(dot)com>
> wrote:
> >> I think you misunderstood me ;). I was talking about the old CF
> >> application providing a RSS feed of all changes to all entries.
> >> https://commitfest-old.postgresql.org/action/commitfest_activity.rss
>
> > Oh, I didn't know this one. That's indeed useful.
>
> Personally, I never used the RSS feed as such, but I did often consult the
> "activity log" pages, which I think are equivalent:
>
> https://commitfest-old.postgresql.org/action/commitfest_activity?id=25
>
> That feature seems to be gone too :-(
>

Activity log is back, clickable from the top right corner when viewing the
frontpage or a commitfest.

RSS feed is there as well.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Tue, Jan 20, 2015 at 1:44 AM, Kyotaro HORIGUCHI <
horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:

> Hello, the new app's clean looking gets my favor and the
> integrated operation will do good for me:)
>
> By the way, I got the following notice when I tried to "Add
> comment" on the new app.
>
> "Note!: This form will generate an email to the public
> mailinglist pgsql-hackers, with sender set to
> horiguchi(dot)kyotaro(at)oss(dot)ntt(dot)co(dot)jp!"
>
> Hmm. The mail address indeed *was* mine but is now obsolete, so
> that the email might bounce. But I haven't find how to change it
> within the app itself, and the PostgreSQL community account page.
>
> https://www.postgresql.org/account/
>
> Could you tell me how do I change the sender address?
>

Hi!

I've just pushed a change to the main website which now lets you change the
email address there. That meas you can go to
https://www.postgresql.org/account/profile/ and change the email. After you
have confirmed the change (an email will be sent to your new address when
you change it), the the new one should be valid on the cf app (if it
doesn't show up, log out of the cf app and back in, and it should show up).

I will look into Andrews issue of being able to have multiple email
addresses soon as well - but one feature per evening :)

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus,

Now that I'm back on this side of the Pacific, is there any additional
data entry/cleanup which needs doing?

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Thu, Jan 22, 2015 at 10:47 AM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> Now that I'm back on this side of the Pacific, is there any additional
> data entry/cleanup which needs doing?
An extra look would be worth it. Magnus or I may have missed patch
entries between the old and new apps.
My2c.
--
Michael

Lovely!

> I've just pushed a change to the main website which now lets you change the
> email address there. That meas you can go to
> https://www.postgresql.org/account/profile/ and change the email. After you
> have confirmed the change (an email will be sent to your new address when
> you change it), the the new one should be valid on the cf app (if it
> doesn't show up, log out of the cf app and back in, and it should show up).
>
> I will look into Andrews issue of being able to have multiple email
> addresses soon as well - but one feature per evening :)

It works perfectly. Thanks.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Tue, Jan 20, 2015 at 10:58 AM, Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk
> wrote:

> >>>>> "Kyotaro" == Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
> writes:
>
> Kyotaro> Hmm. The mail address indeed *was* mine but is now obsolete,
> Kyotaro> so that the email might bounce. But I haven't find how to
> Kyotaro> change it within the app itself, and the PostgreSQL community
> Kyotaro> account page.
>
> Just being able to change the email address on the community account
> isn't enough; I for one am subscribed to the lists with a different
> email address than the one associated with my community account (for
> spam management reasons). There needs to be a way to have multiple
> addresses or to specify which is to be used for the post.
>

This should also be fixed now. You can click Edit Profile, and add one or
more extra email addresses there (after email verification of course), and
then pick one of those to use for sending email.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Hi,

Thanks for the excellent work on the new commitfest app. It looks
awesome so far, though I'm betting the commitfest manager is the one who
reaps the most benefits.

Wanted to highlight this request:

Andres Freund wrote:
> What I'm also missing from the old app is that previously 'reviews'
> could explicitly be linked from the app. Now there's a list of emails in
> the thread, nice!, but in bigger threads that really doesn't help to
> find the actual review.

Note for instance the "BRIN inclusion operator class" patch here,
https://commitfest.postgresql.org/3/31/

The link points to the generic BRIN thread I started (you can see it in
the history). If you list all attachments you can see that all the BRIN
patches are linked; Emre's patch is there, it seems, only because it's
the one most recently posted.

Not sure what to do here, but it's somewhat annoying.

Also, I was pretty used to offline operation with the old one: I could
load the page, for instance
https://commitfest-old.postgresql.org/action/commitfest_view?id=25
and the "patch" links alongside each patch had the message-ids with
which I could search my local mbox. In the new one, the only way I can
get the message-id is by opening the patch page. It would be pretty
useful to have a "copy message-id to clipboard" button, or something
similar, so that I could transfer operation from the web browser to my
mail client. Having one message-id per patch in the commitfest summary
page is enough for my use case (probably pointing to the most recent
attachment in the linked threads.)

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Hi,

seems the CF app uses an invalid e-mail address when sending messages to
pgsql-hackers - I've added a comment to one of the patches and got this:

pgsql-hackers-testing(at)localhost
Unrouteable address

Maybe that's expected as the CF app is new, but I haven't seen it
mentioned in this thread.

regards
Tomas

On Sun, Jan 25, 2015 at 1:28 AM, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
wrote:

> Hi,
>
> seems the CF app uses an invalid e-mail address when sending messages to
> pgsql-hackers - I've added a comment to one of the patches and got this:
>
> pgsql-hackers-testing(at)localhost
> Unrouteable address
>
> Maybe that's expected as the CF app is new, but I haven't seen it
> mentioned in this thread.
>
>

Yikes. That's clearly a testing setting that should not have been included
in the production copy.

Thanks for reporting - fixed!

//Magnus

Is there any possibility of making it possible to "annotate"
particular messages (in particular, patch-related messages) with brief
comments? I would like to be able to highlight particular messages as
particular versions of the patch, and have it be apparent what
properties that version has at a glance.

The mailing list integration is good, but seems like it could often be
overkill. I just want to "tag" an existing message for readability
here, like with the old commitfest app. I like to make things easy to
find from the CF app, particularly for larger, more complicated
things.

Thanks
--
Peter Geoghegan

>>>>> "Peter" == Peter Geoghegan <pg(at)heroku(dot)com> writes:

Peter> The mailing list integration is good, but seems like it could
Peter> often be overkill. I just want to "tag" an existing message for
Peter> readability here, like with the old commitfest app. I like to
Peter> make things easy to find from the CF app, particularly for
Peter> larger, more complicated things.

There's a fairly serious readability problem when someone has posted a
patch as a subthread of some more general discussion. For example, look
at the "adaptive ndistinct estimator" patch: it's not obvious which
attachment is the actual patch, and whether the latest email has
anything to do with the patch is entirely arbitrary.

Either there needs to be an ironclad "always start a new thread for a
new patch" rule, or the app needs to be smarter about threading.

--
Andrew (irc:RhodiumToad)

On Sun, Jan 25, 2015 at 3:22 AM, Andrew Gierth
<andrew(at)tao11(dot)riddles(dot)org(dot)uk> wrote:
> There's a fairly serious readability problem when someone has posted a
> patch as a subthread of some more general discussion. For example, look
> at the "adaptive ndistinct estimator" patch: it's not obvious which
> attachment is the actual patch, and whether the latest email has
> anything to do with the patch is entirely arbitrary.

I think that the inability to put each message in context, with
"metadata" comments associated with individual messages is a serious
regression in functionality. I hope it is fixed soon. I raised this
issue at the earliest opportunity, when Magnus privately sought
feedback early last year.

--
Peter Geoghegan

On Mon, Jan 26, 2015 at 3:13 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> On Sun, Jan 25, 2015 at 3:22 AM, Andrew Gierth
> <andrew(at)tao11(dot)riddles(dot)org(dot)uk> wrote:
>> There's a fairly serious readability problem when someone has posted a
>> patch as a subthread of some more general discussion. For example, look
>> at the "adaptive ndistinct estimator" patch: it's not obvious which
>> attachment is the actual patch, and whether the latest email has
>> anything to do with the patch is entirely arbitrary.
>
> I think that the inability to put each message in context, with
> "metadata" comments associated with individual messages is a serious
> regression in functionality. I hope it is fixed soon. I raised this
> issue at the earliest opportunity, when Magnus privately sought
> feedback early last year.

I agree. Starting a new email thread for each patch version is, IMHO,
a complete non-starter. It's 100% contrary to what has generally been
advocated as best-practice up until now, and it is basically saying we
should alter our workflow because the tool can't cope with the one
we've got. The whole point of having home-grown tools for this stuff
is that they're supposed to work with the way we already like to do
things instead of forcing us to work in new ways.

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

On Mon, Jan 26, 2015 at 9:20 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, Jan 26, 2015 at 3:13 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> > On Sun, Jan 25, 2015 at 3:22 AM, Andrew Gierth
> > <andrew(at)tao11(dot)riddles(dot)org(dot)uk> wrote:
> >> There's a fairly serious readability problem when someone has posted a
> >> patch as a subthread of some more general discussion. For example, look
> >> at the "adaptive ndistinct estimator" patch: it's not obvious which
> >> attachment is the actual patch, and whether the latest email has
> >> anything to do with the patch is entirely arbitrary.
> >
> > I think that the inability to put each message in context, with
> > "metadata" comments associated with individual messages is a serious
> > regression in functionality. I hope it is fixed soon. I raised this
> > issue at the earliest opportunity, when Magnus privately sought
> > feedback early last year.
>

Yes, and the agreement after that feedback was to try it out and then
figure out what changes were needed? As about half the feedback said it was
better without and half said it was better with.

I agree. Starting a new email thread for each patch version is, IMHO,
> a complete non-starter. It's 100% contrary to what has generally been
> advocated as best-practice up until now, and it is basically saying we
> should alter our workflow because the tool can't cope with the one
> we've got. The whole point of having home-grown tools for this stuff
> is that they're supposed to work with the way we already like to do
> things instead of forcing us to work in new ways.
>
>
Why would you create a new thread for a new *version* of a patch? The whole
*point* of the system is that you shouldn't do that, yes, so I'm not sure
where you got the idea that you should do that from?

I though the issue currently discussed was when posted a *different* patch
on the same thread, or that this required the first patch in a thread that
used to be about something that was not a patch.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Mon, Jan 26, 2015 at 3:24 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> Yes, and the agreement after that feedback was to try it out and then figure
> out what changes were needed? As about half the feedback said it was better
> without and half said it was better with.

Well, I can't speak to anyone else's opinion, but I'm quite sure I
raised the issue that we need a way to call out which messages in the
thread are important, and I think that's pretty much what Peter is
saying, too. I find the new tool essentially unusable - having one
link to the whole thread instead of individual links to just the
important messages in that thread is a huge regression for me, as is
the lack of the most recent activity on the summary page. I don't
know how much more feedback than that you need to be convinced, but
I'm going to shut up now before I say something I will later regret.

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

On Mon, Jan 26, 2015 at 12:46 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> Well, I can't speak to anyone else's opinion, but I'm quite sure I
> raised the issue that we need a way to call out which messages in the
> thread are important, and I think that's pretty much what Peter is
> saying, too.

It is.

> I find the new tool essentially unusable - having one
> link to the whole thread instead of individual links to just the
> important messages in that thread is a huge regression for me, as is
> the lack of the most recent activity on the summary page.

Yes, the lack of the most recent activity is also a major omission.

I like some things about the new app. The general idea of having the
mailing list traffic drive the commitfest app is a good one. However,
it seems we've gone too far. I strongly feel that the two regressions
called out here are big problems.
--
Peter Geoghegan

On Mon, Jan 26, 2015 at 9:46 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, Jan 26, 2015 at 3:24 PM, Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> > Yes, and the agreement after that feedback was to try it out and then
> figure
> > out what changes were needed? As about half the feedback said it was
> better
> > without and half said it was better with.
>
> Well, I can't speak to anyone else's opinion, but I'm quite sure I
> raised the issue that we need a way to call out which messages in the
> thread are important, and I think that's pretty much what Peter is
> saying, too. I find the new tool essentially unusable - having one
> link to the whole thread instead of individual links to just the
> important messages in that thread is a huge regression for me, as is
> the lack of the most recent activity on the summary page. I don't
> know how much more feedback than that you need to be convinced, but
> I'm going to shut up now before I say something I will later regret.
>

According to my mailbox, you didn't even respond on that thread. But it may
well be that your email ended up on some other thread and therefor was not
included when I went back and looked over all the responses I got on it. If
that was the case, then I apologize for loosing track of the feedback.

The "most recent activity on the summary page" is on my short-term todo
list to fix. The past couple of days have been a bit too busy to get that
done though, mainly due to the upcoming FOSDEM and pgday events. But rest
assured that part is definitely on the list, as it doesn't actually change
any functionality, it's just a view. Same as that "quick stats numbers"
thing on the frontpage of each cf.

As for being able to flag more things on individual emails/patches, I am
definitely not against that in principle, if that's what people prefer. But
I don't think it's unreasonable to give it a few days and then collect
feedback on that (and other things).

Which of course also includes rolling back the whole thing if people prefer
the older one - that has been an option on the table from the time we
decided to give this one a try in the first place. (Though in that case, we
really need to find a maintainer for that code, as it's we don't seem to
have that now. But I'm sure that can get sorted)

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On 2015-01-26 12:54:04 -0800, Peter Geoghegan wrote:
> On Mon, Jan 26, 2015 at 12:46 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > Well, I can't speak to anyone else's opinion, but I'm quite sure I
> > raised the issue that we need a way to call out which messages in the
> > thread are important, and I think that's pretty much what Peter is
> > saying, too.
>
> It is.

Agreed.

> > I find the new tool essentially unusable - having one
> > link to the whole thread instead of individual links to just the
> > important messages in that thread is a huge regression for me, as is
> > the lack of the most recent activity on the summary page.
>
> Yes, the lack of the most recent activity is also a major omission.

That one is there now though, isn't it?

For specific CF:
https://commitfest.postgresql.org/3/activity/
All CFs:
https://commitfest.postgresql.org/activity/

Or do you mean something else?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Mon, Jan 26, 2015 at 10:05 PM, Andres Freund <andres(at)2ndquadrant(dot)com>
wrote:

> On 2015-01-26 12:54:04 -0800, Peter Geoghegan wrote:
> > On Mon, Jan 26, 2015 at 12:46 PM, Robert Haas <robertmhaas(at)gmail(dot)com>
> wrote:
> > > Well, I can't speak to anyone else's opinion, but I'm quite sure I
> > > raised the issue that we need a way to call out which messages in the
> > > thread are important, and I think that's pretty much what Peter is
> > > saying, too.
> >
> > It is.
>
> Agreed.
>
> > > I find the new tool essentially unusable - having one
> > > link to the whole thread instead of individual links to just the
> > > important messages in that thread is a huge regression for me, as is
> > > the lack of the most recent activity on the summary page.
> >
> > Yes, the lack of the most recent activity is also a major omission.
>
> That one is there now though, isn't it?
>
> For specific CF:
> https://commitfest.postgresql.org/3/activity/
> All CFs:
> https://commitfest.postgresql.org/activity/
>
> Or do you mean something else?
>

I assume what was referred to was that the old cf app would show the last
3 (I think it was) comments/patches/whatnot on a patch on the summary page
(and then clickthrough for more details).

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Mon, Jan 26, 2015 at 4:01 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> According to my mailbox, you didn't even respond on that thread. But it may
> well be that your email ended up on some other thread and therefor was not
> included when I went back and looked over all the responses I got on it. If
> that was the case, then I apologize for loosing track of the feedback.

I remember bringing it up at PGCon, I think. I don't know whether I
wrote it in an email.

> The "most recent activity on the summary page" is on my short-term todo list
> to fix. The past couple of days have been a bit too busy to get that done
> though, mainly due to the upcoming FOSDEM and pgday events. But rest assured
> that part is definitely on the list, as it doesn't actually change any
> functionality, it's just a view. Same as that "quick stats numbers" thing on
> the frontpage of each cf.

OK. What I'd like to understand is why this new app had to be rolled
out before these things were done. We've been using my app for 5
years and it, like, worked. So why the rush to roll it out with these
known issues unfixed? I mean, it's not like you couldn't look at any
time and see what the new app lacked that the old app had. The last
time you presented this app for feedback, which I remember to be
PGCon, it was so buggy that there was no point in trying to form any
considered opinion of it. Now, it's rolled out, but with a bunch of
stuff that people use and rely on missing. I grant that some of those
things you may not have realized anyone cared about, but it feels to
me like this got pushed into production without really getting it to
feature parity. I could've spent more of my time complaining about
that than I did, but why should I have to do that?

> As for being able to flag more things on individual emails/patches, I am
> definitely not against that in principle, if that's what people prefer. But
> I don't think it's unreasonable to give it a few days and then collect
> feedback on that (and other things).

Suit yourself.

> Which of course also includes rolling back the whole thing if people prefer
> the older one - that has been an option on the table from the time we
> decided to give this one a try in the first place. (Though in that case, we
> really need to find a maintainer for that code, as it's we don't seem to
> have that now. But I'm sure that can get sorted)

I understand that having someone who has the time to maintain the code
is an important issue, and I admit I don't, and haven't for a while.
There is a lot of sense in replacing the app with something that uses
the same software framework as the rest of our infrastructure, which I
understand that yours does. And it's not like what the new one does
is horribly different from what the old one did. So I don't see that
there's any reason we can't make the new one work. But I confess to
having no patience with the idea that I have to build a consensus to
get you to re-add features that you removed. You can take that
position, and since you are the tool maintainer it's not exactly
unfair, but since I was willing to put in the work to add those
features in the first place, I probably think they were worth having.

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

On Mon, Jan 26, 2015 at 4:07 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> I assume what was referred to was that the old cf app would show the last 3
> (I think it was) comments/patches/whatnot on a patch on the summary page
> (and then clickthrough for more details).

Yep.

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

On 01/26/2015 12:46 PM, Robert Haas wrote:
> I find the new tool essentially unusable - having one
> link to the whole thread instead of individual links to just the
> important messages in that thread is a huge regression for me, as is
> the lack of the most recent activity on the summary page.

Well, if it's essentially unusable, then we've reached parity with the
old app (yes, you deserved that).

The difference between the old and new apps is that it's actually
*possible* to improve things on the new app, which was never going to
happen on the old one. There's also a significant advantage in knowing
that the entire email thread is available on a patch without someone
having to remember to manually paste in each message ID, something which
failed to happen at least 1/3 of the time for important messages.

What would be cool is a way to "flag" individual messages in the email
thread as being important. Will give it some thought and suggest something.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Mon, Jan 26, 2015 at 4:16 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> Well, if it's essentially unusable, then we've reached parity with the
> old app (yes, you deserved that).

No, I didn't. What we had before I wrote that tool was a bunch of
wiki pages you put together which were forever having problems with
multiple people editing the page at the same time, and not always
adhering to the formatting standards, and sometimes accidentally or
purposefully deleting things other people had done. The tool was
written to mimic that, but without the edit-war chaos. So, if sucked,
it sucked because it mimicked something designed by you. Furthermore,
if it sucked so bad, why did it take anyone 5 years to get around to
rewriting it? It took me less than a year to get around to replacing
what you wrote.

> The difference between the old and new apps is that it's actually
> *possible* to improve things on the new app, which was never going to
> happen on the old one.

That is probably a fair critique.

> There's also a significant advantage in knowing
> that the entire email thread is available on a patch without someone
> having to remember to manually paste in each message ID, something which
> failed to happen at least 1/3 of the time for important messages.

As far as I can see, the new app offers no real advantage in this
regard. The thing that really made things better here is the work
that was done to make threading in our archives work properly. In the
old app, you could click on the last message for which someone put the
message-ID and then go to the end of the thread. In the new app, you
can... pretty much do the same thing. It's not like every message in
the thread shows up in the app. The latest one with an attachment
does, but that's not necessarily the latest patch version.

(While I'm complaining, the links only go to the "flat" version of the
thread, while I happen to prefer the version that shows one message at
a time with a message-ID selector to switch messages.)

> What would be cool is a way to "flag" individual messages in the email
> thread as being important. Will give it some thought and suggest something.

You make that comment as if three people hadn't already +1'd that idea.

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

On 01/26/2015 01:29 PM, Robert Haas wrote:
> Furthermore,
> if it sucked so bad, why did it take anyone 5 years to get around to
> rewriting it? It took me less than a year to get around to replacing
> what you wrote.

Because whoever replaced it knew they'd be facing a shitstorm of criticism?

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On 2015-01-26 13:32:51 -0800, Josh Berkus wrote:
> On 01/26/2015 01:29 PM, Robert Haas wrote:
> > Furthermore,
> > if it sucked so bad, why did it take anyone 5 years to get around to
> > rewriting it? It took me less than a year to get around to replacing
> > what you wrote.
>
> Because whoever replaced it knew they'd be facing a shitstorm of criticism?

Oh, comeon. Grow up.

A missing feature that several people commented on *before* the tool was
released, and that several people have commented upon since isn't a
"shitstorm of criticism".

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Mon, Jan 26, 2015 at 4:32 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> On 01/26/2015 01:29 PM, Robert Haas wrote:
>> Furthermore,
>> if it sucked so bad, why did it take anyone 5 years to get around to
>> rewriting it? It took me less than a year to get around to replacing
>> what you wrote.
>
> Because whoever replaced it knew they'd be facing a shitstorm of criticism?

Didn't stop me. And actually, I didn't face a shitstorm of criticism.
The way I remember it, I got a pretty much unqualified positive
reaction at the time. Only later, when you had conveniently forgotten
how bad things were before we had that tool, did you start routinely
dumping on it. And, AFAICS, not because of anything it did wrong,
only because of things that it didn't do that were features you wanted
to have.

Most of those features were things that I could not possibly have
implemented anyway because they would have required deeper integration
with the authentication system than was possible with the access my
app had. Automatically linking new emails? Not possible: I had no
archives access. Automatic emails to users? Not possible: no access
to email addresses. Place to put a link to a git branch? Yeah, I
could have done that, and just didn't, but the new app doesn't do it
either, yet anyway. I don't know how Magnus's app is connecting in to
the rest of the PG infrastructure, but it's obviously got more access
than mine had. And I think that's great, because hopefully it will
eventually make this much nicer than what I had. It isn't nicer yet,
though, and you showing up and tossing out insults based on
revisionist history won't fix that.

What particularly peeves me about your remarks is that you've
basically made no contribution to the CommitFest process in years. I
am less active than I used to be, but I still do a lot of reviewing
and committing of patches and generally put a pretty significant
amount of time into it. Despite whatever shortcomings that app I
wrote has, so do a lot of other people. The CommitFest process has
got its issues, particularly a lack of reviewers, but it is still
better than having no process, and yet your only contributions seem to
be to denigrate the people who are putting time into it. You're using
the poor quality of my app, and an almost total misinterpretation of
what was said about attracting new reviewers at PGCon, as excuses for
your non-participation, and that is certainly your prerogative. You
have as much right not to participate as anyone. But refusing to
participate except to throw bricks is not going to move this project
forward.

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

Robert,

> Didn't stop me. And actually, I didn't face a shitstorm of criticism.
> The way I remember it, I got a pretty much unqualified positive
> reaction at the time.

Including from me, because it was a huge improvement on what we had
before. As the new app is.

> Only later, when you had conveniently forgotten
> how bad things were before we had that tool, did you start routinely
> dumping on it. And, AFAICS, not because of anything it did wrong,
> only because of things that it didn't do that were features you wanted
> to have.

In order to get a consensus on moving to a new app I had to explain what
was wrong with the old app. Eventually I had to use strong language to
do so, because nobody was paying attention otherwise. While Magnus's
app isn't my original proposal, I'm 100% behind it because it gets us
moving forward and eliminates a lot of obstacles both to submitters and
CFMs. If Magnus hasn't already, in the future we'll be able to add more
features to make things faster for major reviewers as well.

> And I think that's great, because hopefully it will
> eventually make this much nicer than what I had. It isn't nicer yet,
> though, and you showing up and tossing out insults based on
> revisionist history won't fix that.

You didn't previously say "isn't nicer". You said "essentially
unusuable". There's a big difference between those two phrases. Your
emails came off as "the new app is a disaster, we should revert
immediately", and I'm not the only one who read them that way.

If you're going to throw around negative hyperbole, expect to get it
thrown back at you.

> What particularly peeves me about your remarks is that you've
> basically made no contribution to the CommitFest process in years.

Aside from being a CFM for one CF each year, of course.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Mon, Jan 26, 2015 at 5:36 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> In order to get a consensus on moving to a new app I had to explain what
> was wrong with the old app. Eventually I had to use strong language to
> do so, because nobody was paying attention otherwise. While Magnus's
> app isn't my original proposal, I'm 100% behind it because it gets us
> moving forward and eliminates a lot of obstacles both to submitters and
> CFMs. If Magnus hasn't already, in the future we'll be able to add more
> features to make things faster for major reviewers as well.

Frankly, I don't believe that you had much to do with getting
consensus on moving to a new app. I believe Magnus did that, mostly
by writing it. And I'm not complaining about whatever strong language
you may have used in the past; I'm complaining about you showing up on
this thread now, after the switch has already been made, to lob
insults.

>> And I think that's great, because hopefully it will
>> eventually make this much nicer than what I had. It isn't nicer yet,
>> though, and you showing up and tossing out insults based on
>> revisionist history won't fix that.
>
> You didn't previously say "isn't nicer". You said "essentially
> unusuable". There's a big difference between those two phrases. Your
> emails came off as "the new app is a disaster, we should revert
> immediately", and I'm not the only one who read them that way.

I stand by what I said. I find it very hard to use in the present
state for the reasons that I set out. That was not intended as a
request for a revert. I have subsequently written several emails
clarifying my position on that point. If Magnus *wants* to revert it,
that's fine. But I suspect he doesn't, and that's fine too. However,
I'd very much like the features that are missing added back, and,
yeah, I'm annoyed that they are missing.

> If you're going to throw around negative hyperbole, expect to get it
> thrown back at you.

Oh, give me a break.

>> What particularly peeves me about your remarks is that you've
>> basically made no contribution to the CommitFest process in years.
>
> Aside from being a CFM for one CF each year, of course.

OK, perhaps I'm overstating it.

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

On Mon, Jan 26, 2015 at 9:46 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, Jan 26, 2015 at 3:24 PM, Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> > Yes, and the agreement after that feedback was to try it out and then
> figure
> > out what changes were needed? As about half the feedback said it was
> better
> > without and half said it was better with.
>
> Well, I can't speak to anyone else's opinion, but I'm quite sure I
> raised the issue that we need a way to call out which messages in the
> thread are important, and I think that's pretty much what Peter is
> saying, too. I find the new tool essentially unusable - having one
> link to the whole thread instead of individual links to just the
> important messages in that thread is a huge regression for me, as is
> the lack of the most recent activity on the summary page. I don't
> know how much more feedback than that you need to be convinced, but
> I'm going to shut up now before I say something I will later regret.
>
>
So in an attempt to actually move this forward in a constructive way I'm
going to ignore a bunch of what happened after this email, and fork the
discussion at this point.

Since it's pretty clear that several people, many of who are definitely
more productive pg developers than me (and some of the others who
specifically said they did not want this feature), have spoken up about it
*after* the release of it (there's no point in arguing who said what
beforehand), we should look into doing that.

First of all - assuming we'lI fix this particular thing. Are there *other*
things that you would also say makes the tool "essentially unusable"?
Because if there are, can we get that out of the way early? I wouldn't want
to spend a bunch of time fixing this, just to get back into the same round
of argument again, and basically the only thing that's accepted is a
rollback. Because if that's what people want, then let's rollback - I'm not
going to waste time on it in that case. (I'm equally not going to spend
time on fixing anything on the old one anymore, so the next time there's a
security issue or such around it, it will be shut down until someone fixes
it - but that's still better than a tool people don't like to use)

Second, so let's look at how to actually do this.

What kind of annotation are we actually talking about here? Are you looking
for something that's basically "star a message" (like you'd do in a MUA in
a lot of cases)? Or "assign tag" (from a set of - editable of course -
pre-existing tags). Or a complete free-text field?

Then, what do you need to be able to annotate on?

Right now the app only shows messages that actually have attachments on
them, assuming that you'd want to read the whole thread if you want to look
at other things. For this reason, for the message thread itself it only
keeps track of the first and last message (so you can know when it has
updated). Are you saying you want to be able to attach an annotation to
*any* message in the thread? If so, we'd have to pull in and show every
single message on the thread in the UI - I'm thinking that might become
more than a little cluttered in a lot of cases. If that is what you're
talking about, any suggestions for how to actually make that into an UI
that's not that cluttered?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Mon, Jan 26, 2015 at 10:29 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> (While I'm complaining, the links only go to the "flat" version of the
> thread, while I happen to prefer the version that shows one message at
> a time with a message-ID selector to switch messages.)
>

Then you're clicking the wrong link :) As long as you click the link for
the message (first or last) you get the "regular" view. The "thread" link
always goes to the same message as the "first" one, except the flat view.
So you have a choice of both (and these links are both always visible, none
of them is in the "collapsed" area which holds the attachments)

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Mon, Jan 26, 2015 at 10:16 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, Jan 26, 2015 at 4:07 PM, Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> > I assume what was referred to was that the old cf app would show the
> last 3
> > (I think it was) comments/patches/whatnot on a patch on the summary page
> > (and then clickthrough for more details).
>
> Yep.
>
>
So doing this for whatever activity is under patch->history is trivial. Is
that the kind of information that people would like to see? (this assumes
that making annotations etc per the other mesage in this thread will create
an entry in there of course) Or is it something else?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On 2015-02-06 11:51:50 +0100, Magnus Hagander wrote:
> So in an attempt to actually move this forward in a constructive way I'm
> going to ignore a bunch of what happened after this email, and fork the
> discussion at this point.

Sounds good.

> First of all - assuming we'lI fix this particular thing. Are there *other*
> things that you would also say makes the tool "essentially unusable"?

I personally don't have any really fundamental complaints but this
one. I'd like to be able to add some CF app only annotations to the
whole patch, but that's not that important.

> What kind of annotation are we actually talking about here?

Well, to explain where, at least I, am coming from, let's have a look at
an example:
https://commitfest.postgresql.org/4/17/

a) The linked messages aren't going to help either the author, a
committer or prospective reviewers to find existing reviews. And
looking through the whole thread isn't realistic at that message
count. And all of them need to.
b) The 'latest attachement' isn't actually the patch that's supposed to
be reviewed. The first patch is completely outdated. So one would
need to grovel through all the linked patches.
c) By just looking at the CF entry I have no clue what the status of the
patch actually is. Sure it "Needs review", but that can mean minor
changes or major reworks. In the old app the comments to the patches
and reviews could say things like "major issues identified,
significant work needed", "some cosmetic issues remain", "new version
of patch with major redesign" or "rebased patch without other changes".

I don't think any of these questions can be answered by improved
automatisms alone.

This really doesn't matter much on a 5-10 message thread about a simple
patch. But we regularly have thread with hundreds of messages.

Makes sense?

> Are you looking for something that's basically "star a message" (like
> you'd do in a MUA in a lot of cases)? Or "assign tag" (from a set of -
> editable of course - pre-existing tags). Or a complete free-text
> field?

I think it has to be free form. If you look at the above, it's hard to
do that sanely with tags.

> Then, what do you need to be able to annotate on?

I think both individual messages and the CF entry itself, without an
email, make sense.

> Right now the app only shows messages that actually have attachments on
> them, assuming that you'd want to read the whole thread if you want to look
> at other things.

I don't think anybody can realistically do that on more complex
patches. Often the discussion in the beginning has little bearance on
later state, so even if you had the time to do so, it'd better be spent
doing something else.

> Are you saying you want to be able to attach an annotation to *any*
> message in the thread?

Yes.

> If so, we'd have to pull in and show every single message on the
> thread in the UI - I'm thinking that might become more than a little
> cluttered in a lot of cases.

Well, we only would need to show commented on messages in the UI when
viewing the entry.

> If that is what you're talking about, any
> suggestions for how to actually make that into an UI that's not that
> cluttered?

The simplest seems to be to simply allow comments using the message id
:(. I've no really good idea how to allow to select the message to
comment on. Even if we find something else, it should probably be at
least an option - it certainly is quicker for me to grab the message id
of an email I just read or wrote and paste it than pretty much any other
method.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Fri, Feb 6, 2015 at 5:55 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Mon, Jan 26, 2015 at 10:29 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> (While I'm complaining, the links only go to the "flat" version of the
>> thread, while I happen to prefer the version that shows one message at
>> a time with a message-ID selector to switch messages.)
>
> Then you're clicking the wrong link :) As long as you click the link for the
> message (first or last) you get the "regular" view. The "thread" link always
> goes to the same message as the "first" one, except the flat view. So you
> have a choice of both (and these links are both always visible, none of them
> is in the "collapsed" area which holds the attachments)

OK.

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

On Fri, Feb 6, 2015 at 5:51 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> So in an attempt to actually move this forward in a constructive way I'm
> going to ignore a bunch of what happened after this email, and fork the
> discussion at this point.

Thanks, and I probably owe you an apology for some of that, so, sorry
about that.

I think the core of the problem here is that the old application saw
its goal in life as *summarizing* the thread. The idea is that people
would go in and add comments (which could be flagged as comment,
patch, or review) pointing to particularly important messages in the
discussion. The problem with this is that it had to be manually
updated, and some people didn't like that.[1] The new app attaches
the entire thread, which has the advantage that everything is always
there. The problem with that is that the unimportant stuff is there,
too, and there's no way to mark the important stuff so that you can
distinguish between that and the unimportant stuff. I think that's
the problem we need to solve.

One thing that would probably *help* is if the list of attachments
mentioned the names of the files that were attached to each message
rather than just noting that they have some kind of attachment. If
people name their attachments sensibly, then you'll be able to
distinguish parallel-seqscan-v23.patch from
test-case-that-breaks-parallel-seqscan.sql, and that would be nice.
But that doesn't help with, say, distinguishing useful reviews from
general discussion on the thread. I don't think there's any way to do
that in an automated way, so it's got to be something that somebody
does manually.

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

[1] My personal opinion is that this complaint was misguided. I have
yet to see an issue tracker that didn't require scads of manual work
to keep the information in it relevant, and ours had a higher
signal-to-noise ratio than many I've had to use professionally. That
having been said, I understand the frustration.

On Fri, Feb 6, 2015 at 5:20 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Fri, Feb 6, 2015 at 5:51 AM, Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> > So in an attempt to actually move this forward in a constructive way I'm
> > going to ignore a bunch of what happened after this email, and fork the
> > discussion at this point.
>
> Thanks, and I probably owe you an apology for some of that, so, sorry
> about that.
>
> I think the core of the problem here is that the old application saw
> its goal in life as *summarizing* the thread. The idea is that people
> would go in and add comments (which could be flagged as comment,
> patch, or review) pointing to particularly important messages in the
> discussion. The problem with this is that it had to be manually
> updated, and some people didn't like that.[1] The new app attaches
> the entire thread, which has the advantage that everything is always
> there. The problem with that is that the unimportant stuff is there,
> too, and there's no way to mark the important stuff so that you can
> distinguish between that and the unimportant stuff. I think that's
> the problem we need to solve.
>

I'd like the ability to add a comment which does not get turned into an
email.

I liked to add comments which would point out some fact that was important
to testing but which was non-obvious. Often this fact was mentioned
somewhere in the 300 message thread, but it needs to be called out
specifically for people interested in testing but not so interested in
architectural debates. Obviously adding another email to a overly-long
thread is going the wrong way when it comes to making things stand out
better. (Also, if the comment is about a uncommitted dependency, then the
comment can be deleted once the dependency is committed)

>
> One thing that would probably *help* is if the list of attachments
> mentioned the names of the files that were attached to each message
> rather than just noting that they have some kind of attachment. If
> people name their attachments sensibly, then you'll be able to
> distinguish parallel-seqscan-v23.patch from
> test-case-that-breaks-parallel-seqscan.sql, and that would be nice.
>

Yes, I was going to request that as well.

Cheers,

Jeff

On Fri, Feb 6, 2015 at 4:02 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
> I liked to add comments which would point out some fact that was important
> to testing but which was non-obvious. Often this fact was mentioned
> somewhere in the 300 message thread, but it needs to be called out
> specifically for people interested in testing but not so interested in
> architectural debates. Obviously adding another email to a overly-long
> thread is going the wrong way when it comes to making things stand out
> better. (Also, if the comment is about a uncommitted dependency, then the
> comment can be deleted once the dependency is committed)

Good point. I think that your role in testing is very valuable, and
that we should make a particular effort to accommodate that sort of
thing.

--
Peter Geoghegan

On Mon, Jan 19, 2015 at 10:53 PM, Magnus Hagander <magnus(at)hagander(dot)net>
wrote:

> On Mon, Jan 19, 2015 at 10:05 PM, Robert Haas <robertmhaas(at)gmail(dot)com>
> wrote:
>
>> On Mon, Jan 19, 2015 at 3:57 PM, Magnus Hagander <magnus(at)hagander(dot)net>
>> wrote:
>> > The new site has been deployed and should now be usable.
>>
>> There are, for some reason, three copies of "Clarify need for memory
>> barriers in bgworkers" in the in-progress CF. I don't know why that
>> happened, or how to fix it.
>>
>> Also, the old app did this:
>>
>> Status Summary. Needs Review: 34, Waiting on Author: 6, Ready for
>> Committer: 5, Committed: 16, Returned with Feedback: 12, Rejected: 5.
>> Total: 78.
>>
>> ...and you could click on the statuses to filter by that status. I
>> see that this new app has a different way to filter by status, which
>> is fine, but I though the quick summary at the top was awfully useful,
>> so you could see how many total patches there were and how much
>> progress we were making.
>>
>>
> I'll add that to my TODO - but it won't be fixed tonight.
>
>
Status summary is back.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Sat, Feb 7, 2015 at 1:02 AM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:

> On Fri, Feb 6, 2015 at 5:20 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
>> On Fri, Feb 6, 2015 at 5:51 AM, Magnus Hagander <magnus(at)hagander(dot)net>
>> wrote:
>> > So in an attempt to actually move this forward in a constructive way I'm
>> > going to ignore a bunch of what happened after this email, and fork the
>> > discussion at this point.
>>
>> Thanks, and I probably owe you an apology for some of that, so, sorry
>> about that.
>>
>> I think the core of the problem here is that the old application saw
>> its goal in life as *summarizing* the thread. The idea is that people
>> would go in and add comments (which could be flagged as comment,
>> patch, or review) pointing to particularly important messages in the
>> discussion. The problem with this is that it had to be manually
>> updated, and some people didn't like that.[1] The new app attaches
>> the entire thread, which has the advantage that everything is always
>> there. The problem with that is that the unimportant stuff is there,
>> too, and there's no way to mark the important stuff so that you can
>> distinguish between that and the unimportant stuff. I think that's
>> the problem we need to solve.
>>
>
> I'd like the ability to add a comment which does not get turned into an
> email.
>

I really don't ;)

The reason I really don't like that is that this now makes it impossible to
track the review status by just reading throught he mail thread. You have
to context-switch back and forth between the app and the archives. We had
this problem in the old system every now and then where reviews were
posted entirely in the old system...

I liked to add comments which would point out some fact that was important
> to testing but which was non-obvious. Often this fact was mentioned
> somewhere in the 300 message thread, but it needs to be called out
> specifically for people interested in testing but not so interested in
> architectural debates. Obviously adding another email to a overly-long
> thread is going the wrong way when it comes to making things stand out
> better. (Also, if the comment is about a uncommitted dependency, then the
> comment can be deleted once the dependency is committed)
>

Wouldn't that actually be solved if we add this ability to create
"annotations" that would pull int he email in question? If you want to
mainly highlight/call out something specifically for that, it seems like
exactly that feature - add a short annotation and by doing so highlight a
particular email in the thread?

>
>> One thing that would probably *help* is if the list of attachments
>> mentioned the names of the files that were attached to each message
>> rather than just noting that they have some kind of attachment. If
>> people name their attachments sensibly, then you'll be able to
>> distinguish parallel-seqscan-v23.patch from
>> test-case-that-breaks-parallel-seqscan.sql, and that would be nice.
>>
>
> Yes, I was going to request that as well.
>
>
Ok, this is easy enough. There's a field missing in an API call but it
shouldn't be that hard - I'll add this to the short term todo.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> On Sat, Feb 7, 2015 at 1:02 AM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
>> I'd like the ability to add a comment which does not get turned into an
>> email.

> I really don't ;)

> The reason I really don't like that is that this now makes it impossible to
> track the review status by just reading throught he mail thread. You have
> to context-switch back and forth between the app and the archives. We had
> this problem in the old system every now and then where reviews were
> posted entirely in the old system...

Yeah, people did that sometimes and it sucked. At the same time I see
Jeff's point: 300-email threads tend to contain a lot of dross. Could we
address it by allowing only *very short* annotations? The limiting case
would be 1-bit annotations, ie you could star the important messages in a
thread; but that might be too restrictive.

regards, tom lane

On Sat, Feb 7, 2015 at 1:53 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Magnus Hagander <magnus(at)hagander(dot)net> writes:
> > On Sat, Feb 7, 2015 at 1:02 AM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
> >> I'd like the ability to add a comment which does not get turned into an
> >> email.
>
> > I really don't ;)
>
> > The reason I really don't like that is that this now makes it impossible
> to
> > track the review status by just reading throught he mail thread. You have
> > to context-switch back and forth between the app and the archives. We had
> > this problem in the old system every now and then where reviews were
> > posted entirely in the old system...
>
> Yeah, people did that sometimes and it sucked. At the same time I see
> Jeff's point: 300-email threads tend to contain a lot of dross. Could we
> address it by allowing only *very short* annotations? The limiting case
> would be 1-bit annotations, ie you could star the important messages in a
> thread; but that might be too restrictive.
>

Right - to me that's the difference between annotation (per Roberts email
earlier, just "tagging" won't be enough, and I think I agree with that -
but a limited length ones) and a "comment".

It could be that I'm reading too much into Jeff's suggestion though - maybe
that's actually what he is suggesting.

The annotation would then "highlight" the email in the archives with a
direct link (haven't figured out exactly how to implement that part yet but
I have some ideas and I think it's going to work out well).

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Sat, Feb 7, 2015 at 1:46 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:

> On Sat, Feb 7, 2015 at 1:02 AM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
>
>>
>>>
>>> One thing that would probably *help* is if the list of attachments
>>> mentioned the names of the files that were attached to each message
>>> rather than just noting that they have some kind of attachment. If
>>> people name their attachments sensibly, then you'll be able to
>>> distinguish parallel-seqscan-v23.patch from
>>> test-case-that-breaks-parallel-seqscan.sql, and that would be nice.
>>>
>>
>> Yes, I was going to request that as well.
>>
>>
> Ok, this is easy enough. There's a field missing in an API call but it
> shouldn't be that hard - I'll add this to the short term todo.
>
>
Filenames are now shown for attachments, including a direct link to the
attachment itself. I've also run a job to populate all old threads.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Sun, Feb 8, 2015 at 11:04 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> Filenames are now shown for attachments, including a direct link to the
> attachment itself. I've also run a job to populate all old threads.

Thanks!

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

Il 08/02/15 17:04, Magnus Hagander ha scritto:
>
> Filenames are now shown for attachments, including a direct link to the
> attachment itself. I've also run a job to populate all old threads.
>

I wonder what is the algorithm to detect when an attachment is a patch.

If you look at https://commitfest.postgresql.org/4/94/ all the
attachments are marked as "Patch: no", but many of them are
clearly a patch.

Regards,
Marco

--
Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco(dot)nenciarini(at)2ndQuadrant(dot)it | www.2ndQuadrant.it

On Mon, Feb 9, 2015 at 11:09 AM, Marco Nenciarini <
marco(dot)nenciarini(at)2ndquadrant(dot)it> wrote:

> Il 08/02/15 17:04, Magnus Hagander ha scritto:
> >
> > Filenames are now shown for attachments, including a direct link to the
> > attachment itself. I've also run a job to populate all old threads.
> >
>
> I wonder what is the algorithm to detect when an attachment is a patch.
>
> If you look at https://commitfest.postgresql.org/4/94/ all the
> attachments are marked as "Patch: no", but many of them are
> clearly a patch.
>

It uses the "magic" module, same as the "file" command. And that one claims:

mha(at)mha-laptop:/tmp$ file 0003-File-based-incremental-backup-v9.patch
0003-File-based-incremental-backup-v9.patch: ASCII English text, with very
long lines

I think it doesn't consider it a patch because it's not actually a patch -
it looks like a git-format actual email message that *contains* a patch. It
even includes the unix From separator line. So if anything it should have
detected that it's an email message, which it apparently doesn't.

Picking from the very top patch on the cf, an actual patch looks like this:

mha(at)mha-laptop:/tmp$ file psql_fix_uri_service_004.patch
psql_fix_uri_service_004.patch: unified diff output, ASCII text, with very
long lines

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Mon, Feb 9, 2015 at 5:38 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Mon, Feb 9, 2015 at 11:09 AM, Marco Nenciarini
> <marco(dot)nenciarini(at)2ndquadrant(dot)it> wrote:
>>
>> Il 08/02/15 17:04, Magnus Hagander ha scritto:
>> >
>> > Filenames are now shown for attachments, including a direct link to the
>> > attachment itself. I've also run a job to populate all old threads.
>> >
>>
>> I wonder what is the algorithm to detect when an attachment is a patch.
>>
>> If you look at https://commitfest.postgresql.org/4/94/ all the
>> attachments are marked as "Patch: no", but many of them are
>> clearly a patch.
>
> It uses the "magic" module, same as the "file" command. And that one claims:
>
> mha(at)mha-laptop:/tmp$ file 0003-File-based-incremental-backup-v9.patch
> 0003-File-based-incremental-backup-v9.patch: ASCII English text, with very
> long lines
>
> I think it doesn't consider it a patch because it's not actually a patch -
> it looks like a git-format actual email message that *contains* a patch. It
> even includes the unix From separator line. So if anything it should have
> detected that it's an email message, which it apparently doesn't.
>
> Picking from the very top patch on the cf, an actual patch looks like this:
>
> mha(at)mha-laptop:/tmp$ file psql_fix_uri_service_004.patch
> psql_fix_uri_service_004.patch: unified diff output, ASCII text, with very
> long lines

Can we make it smarter, so that the kinds of things people produce
intending for them to be patches are thought by the CF app to be
patches?

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

On Sat, Feb 7, 2015 at 1:59 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:

> On Sat, Feb 7, 2015 at 1:53 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
>> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> > On Sat, Feb 7, 2015 at 1:02 AM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>
>> wrote:
>> >> I'd like the ability to add a comment which does not get turned into an
>> >> email.
>>
>> > I really don't ;)
>>
>> > The reason I really don't like that is that this now makes it
>> impossible to
>> > track the review status by just reading throught he mail thread. You
>> have
>> > to context-switch back and forth between the app and the archives. We
>> had
>> > this problem in the old system every now and then where reviews were
>> > posted entirely in the old system...
>>
>> Yeah, people did that sometimes and it sucked. At the same time I see
>> Jeff's point: 300-email threads tend to contain a lot of dross. Could we
>> address it by allowing only *very short* annotations? The limiting case
>> would be 1-bit annotations, ie you could star the important messages in a
>> thread; but that might be too restrictive.
>>
>
> Right - to me that's the difference between annotation (per Roberts email
> earlier, just "tagging" won't be enough, and I think I agree with that -
> but a limited length ones) and a "comment".
>
> It could be that I'm reading too much into Jeff's suggestion though -
> maybe that's actually what he is suggesting.
>
> The annotation would then "highlight" the email in the archives with a
> direct link (haven't figured out exactly how to implement that part yet but
> I have some ideas and I think it's going to work out well).
>
>
Ok, I've pushed an attempt at doing this.

For each mailthread, you can now create annotations. Each annotation is
connected to a mail in the thread, and has a free text comment field. The
message will then automatically be "highlighted" out of the archives and a
direct link to the message include, alongside the text comment.

I *think* this is approximately what people wanted, so let's give it a
shot. If you have a chance, please test it out and comment as soon as you
can - if I'm completely off track with how it's done, we should back it out
and try a different way before we start putting actual valuable data into
it, so we don't end up with multiple different ways of doing it in the
end...

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Mon, Feb 9, 2015 at 4:56 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, Feb 9, 2015 at 5:38 AM, Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> > On Mon, Feb 9, 2015 at 11:09 AM, Marco Nenciarini
> > <marco(dot)nenciarini(at)2ndquadrant(dot)it> wrote:
> >>
> >> Il 08/02/15 17:04, Magnus Hagander ha scritto:
> >> >
> >> > Filenames are now shown for attachments, including a direct link to
> the
> >> > attachment itself. I've also run a job to populate all old threads.
> >> >
> >>
> >> I wonder what is the algorithm to detect when an attachment is a patch.
> >>
> >> If you look at https://commitfest.postgresql.org/4/94/ all the
> >> attachments are marked as "Patch: no", but many of them are
> >> clearly a patch.
> >
> > It uses the "magic" module, same as the "file" command. And that one
> claims:
> >
> > mha(at)mha-laptop:/tmp$ file 0003-File-based-incremental-backup-v9.patch
> > 0003-File-based-incremental-backup-v9.patch: ASCII English text, with
> very
> > long lines
> >
> > I think it doesn't consider it a patch because it's not actually a patch
> -
> > it looks like a git-format actual email message that *contains* a patch.
> It
> > even includes the unix From separator line. So if anything it should have
> > detected that it's an email message, which it apparently doesn't.
> >
> > Picking from the very top patch on the cf, an actual patch looks like
> this:
> >
> > mha(at)mha-laptop:/tmp$ file psql_fix_uri_service_004.patch
> > psql_fix_uri_service_004.patch: unified diff output, ASCII text, with
> very
> > long lines
>
> Can we make it smarter, so that the kinds of things people produce
> intending for them to be patches are thought by the CF app to be
> patches?
>
>
Doing it wouldn't be too hard, as the code right now is simply:

# Attempt to identify the file using magic information
mtype = mag.buffer(contents)
if mtype.startswith('text/x-diff'):
a.ispatch = True
else:
a.ispatch = False

(where mag is the API call into the magic module)

So we could easily add for example our own regexp parsing or so. The
question is do we want to - because we'll have to maintain it as well. But
I guess if we have a restricted enough set of rules, we can probably live
with that.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Sat, Feb 14, 2015 at 7:29 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> Ok, I've pushed an attempt at doing this.
>
> For each mailthread, you can now create annotations. Each annotation is
> connected to a mail in the thread, and has a free text comment field. The
> message will then automatically be "highlighted" out of the archives and a
> direct link to the message include, alongside the text comment.
>
> I *think* this is approximately what people wanted, so let's give it a shot.
> If you have a chance, please test it out and comment as soon as you can - if
> I'm completely off track with how it's done, we should back it out and try a
> different way before we start putting actual valuable data into it, so we
> don't end up with multiple different ways of doing it in the end...

Hmm. This kind of looks like the right idea, but it's hard to use,
because all you've got to work with is the subject of the message
(which is the same for all), the time it was sent, and the author. In
the old system, you could look at the message in the archives and then
copy-and-paste in the message ID. That's obviously a bit manual, but
it worked.

I suppose what would be really spiffy is if the integration of the
archives and the CF app could be such that you could see the whole
message and then click "annotate this message".

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

On 2/14/15 11:42 AM, Robert Haas wrote:
> On Sat, Feb 14, 2015 at 7:29 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>> Ok, I've pushed an attempt at doing this.
>>
>> For each mailthread, you can now create annotations. Each annotation is
>> connected to a mail in the thread, and has a free text comment field. The
>> message will then automatically be "highlighted" out of the archives and a
>> direct link to the message include, alongside the text comment.
>>
>> I *think* this is approximately what people wanted, so let's give it a shot.
>> If you have a chance, please test it out and comment as soon as you can - if
>> I'm completely off track with how it's done, we should back it out and try a
>> different way before we start putting actual valuable data into it, so we
>> don't end up with multiple different ways of doing it in the end...
>
> Hmm. This kind of looks like the right idea, but it's hard to use,
> because all you've got to work with is the subject of the message
> (which is the same for all), the time it was sent, and the author. In
> the old system, you could look at the message in the archives and then
> copy-and-paste in the message ID. That's obviously a bit manual, but
> it worked.
>
> I suppose what would be really spiffy is if the integration of the
> archives and the CF app could be such that you could see the whole
> message and then click "annotate this message".

It would be nice if CF-related emails had a link at the bottom that took
you to that email in the CF app. Another possibility is some kind of
special markup you could add to an email to create an annotation.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

On Sat, Feb 14, 2015 at 6:42 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Sat, Feb 14, 2015 at 7:29 AM, Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> > Ok, I've pushed an attempt at doing this.
> >
> > For each mailthread, you can now create annotations. Each annotation is
> > connected to a mail in the thread, and has a free text comment field. The
> > message will then automatically be "highlighted" out of the archives and
> a
> > direct link to the message include, alongside the text comment.
> >
> > I *think* this is approximately what people wanted, so let's give it a
> shot.
> > If you have a chance, please test it out and comment as soon as you can
> - if
> > I'm completely off track with how it's done, we should back it out and
> try a
> > different way before we start putting actual valuable data into it, so we
> > don't end up with multiple different ways of doing it in the end...
>
> Hmm. This kind of looks like the right idea, but it's hard to use,
> because all you've got to work with is the subject of the message
> (which is the same for all), the time it was sent, and the author. In
> the old system, you could look at the message in the archives and then
> copy-and-paste in the message ID. That's obviously a bit manual, but
> it worked.
>

Hmm. Agreed, I think my test-thread was just too short to do that.

I've added a box where you can copy/paste the messageid. Back to the
manual, but it works.

> I suppose what would be really spiffy is if the integration of the
> archives and the CF app could be such that you could see the whole
> message and then click "annotate this message".
>
>

That would be quite spiffy. A bit more work though, so that will have to go
on the longer term todo :)

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Sun, Feb 15, 2015 at 8:36 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>> Hmm. This kind of looks like the right idea, but it's hard to use,
>> because all you've got to work with is the subject of the message
>> (which is the same for all), the time it was sent, and the author. In
>> the old system, you could look at the message in the archives and then
>> copy-and-paste in the message ID. That's obviously a bit manual, but
>> it worked.
>
>
> Hmm. Agreed, I think my test-thread was just too short to do that.
>
> I've added a box where you can copy/paste the messageid. Back to the manual,
> but it works.

Thank you for correcting the issue in a timely manner. I appreciate the effort.

--
Peter Geoghegan

On 2/14/15 7:30 AM, Magnus Hagander wrote:
> On Mon, Feb 9, 2015 at 4:56 PM, Robert Haas <robertmhaas(at)gmail(dot)com
> Can we make it smarter, so that the kinds of things people produce
> intending for them to be patches are thought by the CF app to be
> patches?
>
>
> Doing it wouldn't be too hard, as the code right now is simply:
>
> # Attempt to identify the file using magic information
> mtype = mag.buffer(contents)
> if mtype.startswith('text/x-diff'):
> a.ispatch = True
> else:
> a.ispatch = False
>
>
> (where mag is the API call into the magic module)
>
> So we could easily add for example our own regexp parsing or so. The
> question is do we want to - because we'll have to maintain it as well.
> But I guess if we have a restricted enough set of rules, we can probably
> live with that.

As I had described in
<http://www.postgresql.org/message-id/54DD2413.8030201@gmx.net>, this is
all but impossible. The above rule is certainly completely detached
from the reality of what people actually send in. If you are just
ignoring patches that don't match your rule set, this is not going to
work very well.

I think the old system where the patch submitter declared, this message
contains my patch, is the only one that will work.

On Sun, Feb 15, 2015 at 4:59 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> I think the old system where the patch submitter declared, this message
> contains my patch, is the only one that will work.

I tend to agree. That being said, calling out latest attachments is
also useful (or highlighting that a particular mail has a particular
file attached in general).

Annotations-wise, I think that it would be nice to be able to modify
an annotation, in case a typo is made (the old app supported this). I
also think that it's a waste of screen space to show "who" within the
annotation view. Granted, the old app supported this, but I tend to
think that if I actually cared who added a certain annotation, I'd be
happy to drill down into history. I haven't cared so far, AFAICR.

--
Peter Geoghegan

On Sun, Feb 15, 2015 at 7:59 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:

> On 2/14/15 7:30 AM, Magnus Hagander wrote:
> > On Mon, Feb 9, 2015 at 4:56 PM, Robert Haas <robertmhaas(at)gmail(dot)com
> > Can we make it smarter, so that the kinds of things people produce
> > intending for them to be patches are thought by the CF app to be
> > patches?
> >
> >
> > Doing it wouldn't be too hard, as the code right now is simply:
> >
> > # Attempt to identify the file using magic information
> > mtype = mag.buffer(contents)
> > if mtype.startswith('text/x-diff'):
> > a.ispatch = True
> > else:
> > a.ispatch = False
> >
> >
> > (where mag is the API call into the magic module)
> >
> > So we could easily add for example our own regexp parsing or so. The
> > question is do we want to - because we'll have to maintain it as well.
> > But I guess if we have a restricted enough set of rules, we can probably
> > live with that.
>
> As I had described in
> <http://www.postgresql.org/message-id/54DD2413.8030201@gmx.net>, this is
> all but impossible. The above rule is certainly completely detached
> from the reality of what people actually send in. If you are just
> ignoring patches that don't match your rule set, this is not going to
> work very well.
>
> I think the old system where the patch submitter declared, this message
> contains my patch, is the only one that will work.
>

Would you suggest removing the automated system completely, or keep it
around and just make it possible to override it (either by removing the
note that something is a patch, or by making something that's not listed as
a patch become marked as such)?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Sun, Feb 15, 2015 at 9:46 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:

> On Sun, Feb 15, 2015 at 4:59 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> > I think the old system where the patch submitter declared, this message
> > contains my patch, is the only one that will work.
>
> I tend to agree. That being said, calling out latest attachments is
> also useful (or highlighting that a particular mail has a particular
> file attached in general).
>

We can keep listing the attachment and just remove the automated check of
what *kind* of attachment it is.

Annotations-wise, I think that it would be nice to be able to modify
> an annotation, in case a typo is made (the old app supported this). I
>

I was sort of thinking it was something that happened seldom enough that
you could just delete it and remove it again. But I guess if we're
specifically thinking typos, it would make sense to add the ability to edit.

> also think that it's a waste of screen space to show "who" within the
> annotation view. Granted, the old app supported this, but I tend to
> think that if I actually cared who added a certain annotation, I'd be
> happy to drill down into history. I haven't cared so far, AFAICR.
>

Hmm. Personally, I definitely care about who made an annotation, but i
guess I could be OK with going into the history as well. Anybody else have
an opinion on the matter? I don't personally feel strongly either way.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> On Sun, Feb 15, 2015 at 9:46 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>> also think that it's a waste of screen space to show "who" within the
>> annotation view. Granted, the old app supported this, but I tend to
>> think that if I actually cared who added a certain annotation, I'd be
>> happy to drill down into history. I haven't cared so far, AFAICR.

> Hmm. Personally, I definitely care about who made an annotation, but i
> guess I could be OK with going into the history as well. Anybody else have
> an opinion on the matter? I don't personally feel strongly either way.

Hmm ... in the old system, I agree with Peter, in fact I tended to find
the "who" annotations positively misleading, because people frequently
added other peoples' messages to the CF app. It always looked to me like
this had the effect of attributing person A's patch to person B, who'd
actually only updated the CF entry. I think the primary thing you usually
want to know is who is the author of a given email message.

With the new system of auto-adding messages, that particular problem
should be gone, and annotations should be mostly special events rather
than routine maintenance. So maybe who made the annotation is of more
interest than it often was before.

I'm inclined to say you should leave it alone for the moment. We don't
have enough experience with the new system to be sure how we'll use it,
so why do work you might just end up reverting? We can adjust the display
later if we become convinced we don't need this info.

regards, tom lane

On 2/22/15 3:12 PM, Magnus Hagander wrote:
> Would you suggest removing the automated system completely, or keep it
> around and just make it possible to override it (either by removing the
> note that something is a patch, or by making something that's not listed
> as a patch become marked as such)?

I would remove it completely. It has been demonstrated to not work.

Hi,

This thread seems relevant, Please guide me to how can access older CF
pages e.g.

Thread
http://www.postgresql.org/message-id/flat/51F19059(dot)7050702(at)pgexperts(dot)com#51F19059(dot)7050702@pgexperts.com
mentions
following link i.e.

The MSVC portion of this fix got completely lost in the void:
> https://commitfest.postgresql.org/action/patch_view?id=1330

Above link result in the following error i.e.

Not found
> The specified URL was not found.
>

Please do let me know if I missed something. Thanks.

Regards,
Muhammad Asif Naeem

On Tue, Feb 24, 2015 at 11:59 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:

> On 2/22/15 3:12 PM, Magnus Hagander wrote:
> > Would you suggest removing the automated system completely, or keep it
> > around and just make it possible to override it (either by removing the
> > note that something is a patch, or by making something that's not listed
> > as a patch become marked as such)?
>
> I would remove it completely. It has been demonstrated to not work.
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

On Thu, Feb 26, 2015 at 9:15 PM, Asif Naeem <anaeem(dot)it(at)gmail(dot)com> wrote:
> This thread seems relevant, Please guide me to how can access older CF pages
>> The MSVC portion of this fix got completely lost in the void:
>> https://commitfest.postgresql.org/action/patch_view?id=1330
>
> Above link result in the following error i.e.
>
>> Not found
>> The specified URL was not found.
>
> Please do let me know if I missed something. Thanks.

Try commitfest-old instead, that is where the past CF app stores its
data, like that:
https://commitfest-old.postgresql.org/action/patch_view?id=1330
--
Michael

On 02/26/2015 01:59 PM, Michael Paquier wrote:
> On Thu, Feb 26, 2015 at 9:15 PM, Asif Naeem <anaeem(dot)it(at)gmail(dot)com> wrote:
>> This thread seems relevant, Please guide me to how can access older CF pages
>>> The MSVC portion of this fix got completely lost in the void:
>>> https://commitfest.postgresql.org/action/patch_view?id=1330
>>
>> Above link result in the following error i.e.
>>
>>> Not found
>>> The specified URL was not found.
>>
>> Please do let me know if I missed something. Thanks.
>
> Try commitfest-old instead, that is where the past CF app stores its
> data, like that:
> https://commitfest-old.postgresql.org/action/patch_view?id=1330

hmm maybe we should have some sort of handler the redirects/reverse
proxies to the old commitfest app for this.

Stefan

Thank you Michael, It works.

On Thu, Feb 26, 2015 at 5:59 PM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:

> On Thu, Feb 26, 2015 at 9:15 PM, Asif Naeem <anaeem(dot)it(at)gmail(dot)com> wrote:
> > This thread seems relevant, Please guide me to how can access older CF
> pages
> >> The MSVC portion of this fix got completely lost in the void:
> >> https://commitfest.postgresql.org/action/patch_view?id=1330
> >
> > Above link result in the following error i.e.
> >
> >> Not found
> >> The specified URL was not found.
> >
> > Please do let me know if I missed something. Thanks.
>
> Try commitfest-old instead, that is where the past CF app stores its
> data, like that:
> https://commitfest-old.postgresql.org/action/patch_view?id=1330
> --
> Michael
>

On Fri, Feb 27, 2015 at 11:32 AM, Stefan Kaltenbrunner <
stefan(at)kaltenbrunner(dot)cc> wrote:

> On 02/26/2015 01:59 PM, Michael Paquier wrote:
> > On Thu, Feb 26, 2015 at 9:15 PM, Asif Naeem <anaeem(dot)it(at)gmail(dot)com> wrote:
> >> This thread seems relevant, Please guide me to how can access older CF
> pages
> >>> The MSVC portion of this fix got completely lost in the void:
> >>> https://commitfest.postgresql.org/action/patch_view?id=1330
> >>
> >> Above link result in the following error i.e.
> >>
> >>> Not found
> >>> The specified URL was not found.
> >>
> >> Please do let me know if I missed something. Thanks.
> >
> > Try commitfest-old instead, that is where the past CF app stores its
> > data, like that:
> > https://commitfest-old.postgresql.org/action/patch_view?id=1330
>
> hmm maybe we should have some sort of handler the redirects/reverse
> proxies to the old commitfest app for this.
>

1+ for seamless integration, at least error message seems require to be
more helpful. Thanks.

> Stefan
>

On Sun, Feb 22, 2015 at 03:12:12PM -0500, Magnus Hagander wrote:
> >                 # Attempt to identify the file using magic information
> >                 mtype = mag.buffer(contents)
> >                 if mtype.startswith('text/x-diff'):
> >                         a.ispatch = True
> >                 else:
> >                         a.ispatch = False
...
>
> I think the old system where the patch submitter declared, this message
> contains my patch, is the only one that will work.
>
>
>
> Would you suggest removing the automated system completely, or keep it around
> and just make it possible to override it (either by removing the note that
> something is a patch, or by making something that's not listed as a patch
> become marked as such)?

One counter-idea would be to assume every attachment is a patch _unless_
the attachment type matches a pattern that identifies it as not a patch.

However, I agree with Tom that we should go a little longer before
changing it.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On Tue, Mar 3, 2015 at 10:58:28AM -0500, Bruce Momjian wrote:
> > Would you suggest removing the automated system completely, or keep it around
> > and just make it possible to override it (either by removing the note that
> > something is a patch, or by making something that's not listed as a patch
> > become marked as such)?
>
> One counter-idea would be to assume every attachment is a patch _unless_
> the attachment type matches a pattern that identifies it as not a patch.
>
> However, I agree with Tom that we should go a little longer before
> changing it.

Also, can we look inside the attachment to see if it starts with
'diff<space>'.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On 3/3/15 11:11 AM, Bruce Momjian wrote:
> On Tue, Mar 3, 2015 at 10:58:28AM -0500, Bruce Momjian wrote:
>>> Would you suggest removing the automated system completely, or keep it around
>>> and just make it possible to override it (either by removing the note that
>>> something is a patch, or by making something that's not listed as a patch
>>> become marked as such)?
>>
>> One counter-idea would be to assume every attachment is a patch _unless_
>> the attachment type matches a pattern that identifies it as not a patch.
>>
>> However, I agree with Tom that we should go a little longer before
>> changing it.
>
> Also, can we look inside the attachment to see if it starts with
> 'diff<space>'.

There are two different issues here: One, how you detect a patch. Two,
whether the latest "patch" is the patch of record for the commit fest item.

I think they are both currently wrong, but the second one is more
important. Of course, if you punt on the first one, you have to do the
second one differently.

On Sun, Feb 22, 2015 at 12:13 PM, Magnus Hagander <magnus(at)hagander(dot)net>
wrote:

> On Sun, Feb 15, 2015 at 9:46 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>
>> On Sun, Feb 15, 2015 at 4:59 PM, Peter Eisentraut <peter_e(at)gmx(dot)net>
>> wrote:
>> > I think the old system where the patch submitter declared, this message
>> > contains my patch, is the only one that will work.
>>
>> I tend to agree. That being said, calling out latest attachments is
>> also useful (or highlighting that a particular mail has a particular
>> file attached in general).
>>
>
> We can keep listing the attachment and just remove the automated check of
> what *kind* of attachment it is.
>

But when an email has multiple attachments, there should be some kind of
indication that this is the case.

In https://commitfest.postgresql.org/4/21/

The line:

Attachment (attlognum-test.sql
<http://www.postgresql.org/message-id/attachment/36971/attlognum-test.sql>)
at 2015-02-23 23:09:06
<http://www.postgresql.org/message-id/54EBB312.7090000@2ndquadrant.com/> from
Tomas Vondra <tomas.vondra at 2ndquadrant.com> (Patch: No)

is misleading.

Cheers,

Jeff