Re: [HACKERS] New CF app deployment

2015-03-13 Thread Jeff Janes
On Sun, Feb 22, 2015 at 12:13 PM, Magnus Hagander mag...@hagander.net
wrote:

 On Sun, Feb 15, 2015 at 9:46 PM, Peter Geoghegan p...@heroku.com wrote:

 On Sun, Feb 15, 2015 at 4:59 PM, Peter Eisentraut pete...@gmx.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.7090...@2ndquadrant.com/ from
Tomas Vondra tomas.vondra at 2ndquadrant.com (Patch: No)

is misleading.

Cheers,

Jeff


Re: [HACKERS] New CF app deployment

2015-03-04 Thread Peter Eisentraut
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
 'diffspace'.

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.



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


Re: [HACKERS] New CF app deployment

2015-03-03 Thread Bruce Momjian
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  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


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


Re: [HACKERS] New CF app deployment

2015-03-03 Thread Bruce Momjian
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
'diffspace'.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


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


Re: [HACKERS] New CF app deployment

2015-02-27 Thread Asif Naeem
On Fri, Feb 27, 2015 at 11:32 AM, Stefan Kaltenbrunner 
ste...@kaltenbrunner.cc wrote:

 On 02/26/2015 01:59 PM, Michael Paquier wrote:
  On Thu, Feb 26, 2015 at 9:15 PM, Asif Naeem anaeem...@gmail.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



Re: [HACKERS] New CF app deployment

2015-02-27 Thread Asif Naeem
Thank you Michael, It works.

On Thu, Feb 26, 2015 at 5:59 PM, Michael Paquier michael.paqu...@gmail.com
wrote:

 On Thu, Feb 26, 2015 at 9:15 PM, Asif Naeem anaeem...@gmail.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



Re: [HACKERS] New CF app deployment

2015-02-26 Thread Stefan Kaltenbrunner
On 02/26/2015 01:59 PM, Michael Paquier wrote:
 On Thu, Feb 26, 2015 at 9:15 PM, Asif Naeem anaeem...@gmail.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


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


Re: [HACKERS] New CF app deployment

2015-02-26 Thread Michael Paquier
On Thu, Feb 26, 2015 at 9:15 PM, Asif Naeem anaeem...@gmail.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


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


Re: [HACKERS] New CF app deployment

2015-02-26 Thread Asif Naeem
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.7050...@pgexperts.com#51f19059.7050...@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 pete...@gmx.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@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers



Re: [HACKERS] New CF app deployment

2015-02-24 Thread Peter Eisentraut
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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] New CF app deployment

2015-02-22 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 On Sun, Feb 15, 2015 at 9:46 PM, Peter Geoghegan p...@heroku.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


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


Re: [HACKERS] New CF app deployment

2015-02-22 Thread Magnus Hagander
On Sun, Feb 15, 2015 at 7:59 PM, Peter Eisentraut pete...@gmx.net wrote:

 On 2/14/15 7:30 AM, Magnus Hagander wrote:
  On Mon, Feb 9, 2015 at 4:56 PM, Robert Haas robertmh...@gmail.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.8030...@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/


Re: [HACKERS] New CF app deployment

2015-02-22 Thread Magnus Hagander
On Sun, Feb 15, 2015 at 9:46 PM, Peter Geoghegan p...@heroku.com wrote:

 On Sun, Feb 15, 2015 at 4:59 PM, Peter Eisentraut pete...@gmx.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/


Re: [HACKERS] New CF app deployment

2015-02-15 Thread Magnus Hagander
On Sat, Feb 14, 2015 at 6:42 PM, Robert Haas robertmh...@gmail.com wrote:

 On Sat, Feb 14, 2015 at 7:29 AM, Magnus Hagander mag...@hagander.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/


Re: [HACKERS] New CF app deployment

2015-02-15 Thread Peter Eisentraut
On 2/14/15 7:30 AM, Magnus Hagander wrote:
 On Mon, Feb 9, 2015 at 4:56 PM, Robert Haas robertmh...@gmail.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.8030...@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.



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


Re: [HACKERS] New CF app deployment

2015-02-15 Thread Peter Geoghegan
On Sun, Feb 15, 2015 at 8:36 AM, Magnus Hagander mag...@hagander.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


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


Re: [HACKERS] New CF app deployment

2015-02-15 Thread Peter Geoghegan
On Sun, Feb 15, 2015 at 4:59 PM, Peter Eisentraut pete...@gmx.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


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


Re: [HACKERS] New CF app deployment

2015-02-14 Thread Magnus Hagander
On Mon, Feb 9, 2015 at 4:56 PM, Robert Haas robertmh...@gmail.com wrote:

 On Mon, Feb 9, 2015 at 5:38 AM, Magnus Hagander mag...@hagander.net
 wrote:
  On Mon, Feb 9, 2015 at 11:09 AM, Marco Nenciarini
  marco.nenciar...@2ndquadrant.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@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@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/


Re: [HACKERS] New CF app deployment

2015-02-14 Thread Magnus Hagander
On Sat, Feb 7, 2015 at 1:59 PM, Magnus Hagander mag...@hagander.net wrote:

 On Sat, Feb 7, 2015 at 1:53 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 Magnus Hagander mag...@hagander.net writes:
  On Sat, Feb 7, 2015 at 1:02 AM, Jeff Janes jeff.ja...@gmail.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/


Re: [HACKERS] New CF app deployment

2015-02-14 Thread Robert Haas
On Sat, Feb 14, 2015 at 7:29 AM, Magnus Hagander mag...@hagander.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


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


Re: [HACKERS] New CF app deployment

2015-02-14 Thread Jim Nasby

On 2/14/15 11:42 AM, Robert Haas wrote:

On Sat, Feb 14, 2015 at 7:29 AM, Magnus Hagander mag...@hagander.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


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


Re: [HACKERS] New CF app deployment

2015-02-09 Thread Robert Haas
On Mon, Feb 9, 2015 at 5:38 AM, Magnus Hagander mag...@hagander.net wrote:
 On Mon, Feb 9, 2015 at 11:09 AM, Marco Nenciarini
 marco.nenciar...@2ndquadrant.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@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@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


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


Re: [HACKERS] New CF app deployment

2015-02-09 Thread Magnus Hagander
On Mon, Feb 9, 2015 at 11:09 AM, Marco Nenciarini 
marco.nenciar...@2ndquadrant.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@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@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/


Re: [HACKERS] New CF app deployment

2015-02-09 Thread Marco Nenciarini
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.nenciar...@2ndquadrant.it | www.2ndQuadrant.it



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] New CF app deployment

2015-02-08 Thread Magnus Hagander
On Sat, Feb 7, 2015 at 1:46 PM, Magnus Hagander mag...@hagander.net wrote:

 On Sat, Feb 7, 2015 at 1:02 AM, Jeff Janes jeff.ja...@gmail.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/


Re: [HACKERS] New CF app deployment

2015-02-08 Thread Robert Haas
On Sun, Feb 8, 2015 at 11:04 AM, Magnus Hagander mag...@hagander.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


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


Re: [HACKERS] New CF app deployment

2015-02-07 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 On Sat, Feb 7, 2015 at 1:02 AM, Jeff Janes jeff.ja...@gmail.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


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


Re: [HACKERS] New CF app deployment

2015-02-07 Thread Magnus Hagander
On Sat, Feb 7, 2015 at 1:53 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 Magnus Hagander mag...@hagander.net writes:
  On Sat, Feb 7, 2015 at 1:02 AM, Jeff Janes jeff.ja...@gmail.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/


Re: [HACKERS] New CF app deployment

2015-02-07 Thread Magnus Hagander
On Sat, Feb 7, 2015 at 1:02 AM, Jeff Janes jeff.ja...@gmail.com wrote:

 On Fri, Feb 6, 2015 at 5:20 AM, Robert Haas robertmh...@gmail.com wrote:

 On Fri, Feb 6, 2015 at 5:51 AM, Magnus Hagander mag...@hagander.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/


Re: [HACKERS] New CF app deployment

2015-02-07 Thread Magnus Hagander
On Mon, Jan 19, 2015 at 10:53 PM, Magnus Hagander mag...@hagander.net
wrote:

 On Mon, Jan 19, 2015 at 10:05 PM, Robert Haas robertmh...@gmail.com
 wrote:

 On Mon, Jan 19, 2015 at 3:57 PM, Magnus Hagander mag...@hagander.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/


Re: [HACKERS] New CF app deployment

2015-02-06 Thread Magnus Hagander
On Mon, Jan 26, 2015 at 10:16 PM, Robert Haas robertmh...@gmail.com wrote:

 On Mon, Jan 26, 2015 at 4:07 PM, Magnus Hagander mag...@hagander.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/


Re: [HACKERS] New CF app deployment

2015-02-06 Thread Magnus Hagander
On Mon, Jan 26, 2015 at 9:46 PM, Robert Haas robertmh...@gmail.com wrote:

 On Mon, Jan 26, 2015 at 3:24 PM, Magnus Hagander mag...@hagander.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/


Re: [HACKERS] New CF app deployment

2015-02-06 Thread Magnus Hagander
On Mon, Jan 26, 2015 at 10:29 PM, Robert Haas robertmh...@gmail.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/


Re: [HACKERS] New CF app deployment

2015-02-06 Thread Andres Freund
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


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


Re: [HACKERS] New CF app deployment

2015-02-06 Thread Robert Haas
On Fri, Feb 6, 2015 at 5:51 AM, Magnus Hagander mag...@hagander.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.


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


Re: [HACKERS] New CF app deployment

2015-02-06 Thread Robert Haas
On Fri, Feb 6, 2015 at 5:55 AM, Magnus Hagander mag...@hagander.net wrote:
 On Mon, Jan 26, 2015 at 10:29 PM, Robert Haas robertmh...@gmail.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


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


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Robert Haas
On Mon, Jan 26, 2015 at 3:13 PM, Peter Geoghegan p...@heroku.com wrote:
 On Sun, Jan 25, 2015 at 3:22 AM, Andrew Gierth
 and...@tao11.riddles.org.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


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


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Magnus Hagander
On Mon, Jan 26, 2015 at 9:20 PM, Robert Haas robertmh...@gmail.com wrote:

 On Mon, Jan 26, 2015 at 3:13 PM, Peter Geoghegan p...@heroku.com wrote:
  On Sun, Jan 25, 2015 at 3:22 AM, Andrew Gierth
  and...@tao11.riddles.org.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/


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Peter Geoghegan
On Sun, Jan 25, 2015 at 3:22 AM, Andrew Gierth
and...@tao11.riddles.org.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


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


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Robert Haas
On Mon, Jan 26, 2015 at 4:16 PM, Josh Berkus j...@agliodbs.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


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


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Josh Berkus
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


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


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Magnus Hagander
On Mon, Jan 26, 2015 at 9:46 PM, Robert Haas robertmh...@gmail.com wrote:

 On Mon, Jan 26, 2015 at 3:24 PM, Magnus Hagander mag...@hagander.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/


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Andres Freund
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


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


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Peter Geoghegan
On Mon, Jan 26, 2015 at 12:46 PM, Robert Haas robertmh...@gmail.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


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


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Robert Haas
On Mon, Jan 26, 2015 at 4:07 PM, Magnus Hagander mag...@hagander.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


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


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Josh Berkus
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


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


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Robert Haas
On Mon, Jan 26, 2015 at 4:01 PM, Magnus Hagander mag...@hagander.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


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


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Andres Freund
On 2015-01-26 12:54:04 -0800, Peter Geoghegan wrote:
 On Mon, Jan 26, 2015 at 12:46 PM, Robert Haas robertmh...@gmail.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


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


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Magnus Hagander
On Mon, Jan 26, 2015 at 10:05 PM, Andres Freund and...@2ndquadrant.com
wrote:

 On 2015-01-26 12:54:04 -0800, Peter Geoghegan wrote:
  On Mon, Jan 26, 2015 at 12:46 PM, Robert Haas robertmh...@gmail.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/


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Robert Haas
On Mon, Jan 26, 2015 at 4:32 PM, Josh Berkus j...@agliodbs.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


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


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Robert Haas
On Mon, Jan 26, 2015 at 5:36 PM, Josh Berkus j...@agliodbs.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


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


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Josh Berkus
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


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


Re: [HACKERS] New CF app deployment

2015-01-26 Thread Robert Haas
On Mon, Jan 26, 2015 at 3:24 PM, Magnus Hagander mag...@hagander.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


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


Re: [HACKERS] New CF app deployment

2015-01-25 Thread Magnus Hagander
On Sun, Jan 25, 2015 at 1:28 AM, Tomas Vondra tomas.von...@2ndquadrant.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@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


Re: [HACKERS] New CF app deployment

2015-01-25 Thread Peter Geoghegan
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


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


Re: [HACKERS] New CF app deployment

2015-01-25 Thread Andrew Gierth
 Peter == Peter Geoghegan p...@heroku.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)


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


Re: [HACKERS] New CF app deployment

2015-01-24 Thread Tomas Vondra
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@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


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


Re: [HACKERS] New CF app deployment

2015-01-22 Thread Alvaro Herrera

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 Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training  Services


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


Re: [HACKERS] New CF app deployment

2015-01-21 Thread Josh Berkus
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


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


Re: [HACKERS] New CF app deployment

2015-01-21 Thread Michael Paquier
On Thu, Jan 22, 2015 at 10:47 AM, Josh Berkus j...@agliodbs.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


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


Re: [HACKERS] New CF app deployment

2015-01-20 Thread Tom Lane
Michael Paquier michael.paqu...@gmail.com writes:
 On Tue, Jan 20, 2015 at 4:31 PM, Andres Freund and...@2ndquadrant.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


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


Re: [HACKERS] New CF app deployment

2015-01-20 Thread Magnus Hagander
On Tue, Jan 20, 2015 at 3:43 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 Michael Paquier michael.paqu...@gmail.com writes:
  On Tue, Jan 20, 2015 at 4:31 PM, Andres Freund and...@2ndquadrant.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/


Re: [HACKERS] New CF app deployment

2015-01-20 Thread Pavel Stehule
2015-01-20 19:16 GMT+01:00 Pavel Stehule pavel.steh...@gmail.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 mag...@hagander.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/





Re: [HACKERS] New CF app deployment

2015-01-20 Thread Pavel Stehule
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 mag...@hagander.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/



Re: [HACKERS] New CF app deployment

2015-01-20 Thread Magnus Hagander
On Tue, Jan 20, 2015 at 3:43 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 Michael Paquier michael.paqu...@gmail.com writes:
  On Tue, Jan 20, 2015 at 4:31 PM, Andres Freund and...@2ndquadrant.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/


Re: [HACKERS] New CF app deployment

2015-01-19 Thread Andres Freund
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


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


Re: [HACKERS] New CF app deployment

2015-01-19 Thread Michael Paquier
On Tue, Jan 20, 2015 at 6:54 AM, Magnus Hagander mag...@hagander.net wrote:
 On Mon, Jan 19, 2015 at 10:10 PM, Robert Haas robertmh...@gmail.com wrote:

 On Mon, Jan 19, 2015 at 4:05 PM, Robert Haas robertmh...@gmail.com
 wrote:
  On Mon, Jan 19, 2015 at 3:57 PM, Magnus Hagander mag...@hagander.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


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


Re: [HACKERS] New CF app deployment

2015-01-19 Thread Michael Paquier
On Tue, Jan 20, 2015 at 10:09 AM, Andres Freund and...@2ndquadrant.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.gd2...@awork2.anarazel.de. I also
recall that you and Robert were marked as reviewers, right?
-- 
Michael


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


Re: [HACKERS] New CF app deployment

2015-01-19 Thread Michael Paquier
On Tue, Jan 20, 2015 at 4:31 PM, Andres Freund and...@2ndquadrant.com wrote:
 On 2015-01-20 11:05:43 +0900, Michael Paquier wrote:
 On Tue, Jan 20, 2015 at 10:09 AM, Andres Freund and...@2ndquadrant.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.gd2...@awork2.anarazel.de. 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


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


Re: [HACKERS] New CF app deployment

2015-01-19 Thread Andres Freund
On 2015-01-20 11:05:43 +0900, Michael Paquier wrote:
 On Tue, Jan 20, 2015 at 10:09 AM, Andres Freund and...@2ndquadrant.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.gd2...@awork2.anarazel.de. 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


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


Re: [HACKERS] New CF app deployment

2015-01-19 Thread Magnus Hagander
On Mon, Jan 19, 2015 at 10:10 PM, Robert Haas robertmh...@gmail.com wrote:

 On Mon, Jan 19, 2015 at 4:05 PM, Robert Haas robertmh...@gmail.com
 wrote:
  On Mon, Jan 19, 2015 at 3:57 PM, Magnus Hagander mag...@hagander.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/


Re: [HACKERS] New CF app deployment

2015-01-19 Thread Magnus Hagander
On Mon, Jan 19, 2015 at 10:01 PM, Robert Haas robertmh...@gmail.com wrote:

 On Mon, Jan 19, 2015 at 3:57 PM, Magnus Hagander mag...@hagander.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/


Re: [HACKERS] New CF app deployment

2015-01-19 Thread Robert Haas
On Mon, Jan 19, 2015 at 4:05 PM, Robert Haas robertmh...@gmail.com wrote:
 On Mon, Jan 19, 2015 at 3:57 PM, Magnus Hagander mag...@hagander.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


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


Re: [HACKERS] New CF app deployment

2015-01-19 Thread Robert Haas
On Mon, Jan 19, 2015 at 3:57 PM, Magnus Hagander mag...@hagander.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


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


Re: [HACKERS] New CF app deployment

2015-01-18 Thread Michael Paquier
On Tue, Jan 13, 2015 at 2:35 PM, Magnus Hagander mag...@hagander.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


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


[HACKERS] New CF app deployment

2015-01-12 Thread Magnus Hagander
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/