On Tue, Jan 28, 2014 at 06:30:40PM +0100, Andres Freund wrote:
> > OK, reverted. I have to question how well-balanced we are when a word
> > change in a C comment can cause so much contention.
>
> The question is rather why to do such busywork changes in the first
> place imo. Without ever lookin
Bruce Momjian escribió:
> On Tue, Jan 28, 2014 at 11:20:39AM -0500, Tom Lane wrote:
> > Andres Freund writes:
> > > On 2014-01-28 11:14:49 -0500, Bruce Momjian wrote:
> > >> OK, so does anyone object to removing this comment line?
> >
> > > Let's just not do anything. This is change for changes s
On 2014-01-28 12:29:25 -0500, Bruce Momjian wrote:
> On Tue, Jan 28, 2014 at 02:25:51PM -0300, Alvaro Herrera wrote:
> > Bruce Momjian escribió:
> > > On Tue, Jan 28, 2014 at 11:20:39AM -0500, Tom Lane wrote:
> > > > Andres Freund writes:
> > > > > On 2014-01-28 11:14:49 -0500, Bruce Momjian wrote
On Tue, Jan 28, 2014 at 02:25:51PM -0300, Alvaro Herrera wrote:
> Bruce Momjian escribió:
> > On Tue, Jan 28, 2014 at 11:20:39AM -0500, Tom Lane wrote:
> > > Andres Freund writes:
> > > > On 2014-01-28 11:14:49 -0500, Bruce Momjian wrote:
> > > >> OK, so does anyone object to removing this comment
On Tue, Jan 28, 2014 at 11:20:39AM -0500, Tom Lane wrote:
> Andres Freund writes:
> > On 2014-01-28 11:14:49 -0500, Bruce Momjian wrote:
> >> OK, so does anyone object to removing this comment line?
>
> > Let's just not do anything. This is change for changes sake. Not
> > improving anything the
Andres Freund writes:
> On 2014-01-28 11:14:49 -0500, Bruce Momjian wrote:
>> OK, so does anyone object to removing this comment line?
> Let's just not do anything. This is change for changes sake. Not
> improving anything the slightest.
Indeed. I'd actually request that you revert your previou
On 2014-01-28 11:14:49 -0500, Bruce Momjian wrote:
> On Mon, Jan 27, 2014 at 02:51:59PM -0800, Kevin Grittner wrote:
> > So anyway, *I* would object to applying that; it was meant to
> > illustrate what the comment, if any, should cover; not to be an
> > actual code change. I don't think the chang
On Mon, Jan 27, 2014 at 02:51:59PM -0800, Kevin Grittner wrote:
> So anyway, *I* would object to applying that; it was meant to
> illustrate what the comment, if any, should cover; not to be an
> actual code change. I don't think the change that was pushed helps
> that comment carry its own weight
Bruce Momjian wrote:
> On Tue, Jun 18, 2013 at 11:04:25AM -0700, Kevin Grittner wrote:
>> D'Arcy J.M. Cain
>>
>>> Although, the more I think about it, the more I think that the
>>> comment is both confusing and superfluous. The code itself is
>>> much clearer.
>>
>> Seriously, if there is any co
On 2014-01-25 17:15:01 -0500, Bruce Momjian wrote:
> On Sat, Jan 25, 2014 at 04:56:37PM -0500, Tom Lane wrote:
> > Bruce Momjian writes:
> > > On Sat, Jan 25, 2014 at 10:40:28PM +0100, Andres Freund wrote:
> > >> I think this style of pinhole copy editing is pretty pointless. There's
> > >> dozen
On Sat, Jan 25, 2014 at 04:56:37PM -0500, Tom Lane wrote:
> Bruce Momjian writes:
> > On Sat, Jan 25, 2014 at 10:40:28PM +0100, Andres Freund wrote:
> >> I think this style of pinhole copy editing is pretty pointless. There's
> >> dozen checks just like this around. If somebody wants to change the
Bruce Momjian writes:
> On Sat, Jan 25, 2014 at 10:40:28PM +0100, Andres Freund wrote:
>> I think this style of pinhole copy editing is pretty pointless. There's
>> dozen checks just like this around. If somebody wants to change the rules
>> or improve comment it takes more than picking a random o
Andres Freund writes:
> On 2014-01-25 16:28:09 -0500, Bruce Momjian wrote:
>> Is everyone OK with me applying this patch from Kevin, attached?
> No. I still think this is stupid. Not at all clearer and possibly breaks
> stuff.
I agree; this patch is flat out wrong. It converts a valid situation
On Sat, Jan 25, 2014 at 10:40:28PM +0100, Andres Freund wrote:
> I don't think it improves things relevantly, but it doesn't make
> anything worse either. So if that makes anybody happy...
>
> I think this style of pinhole copy editing is pretty pointless. There's
> dozen checks just like this aro
On 2014-01-25 16:33:16 -0500, Bruce Momjian wrote:
> On Sat, Jan 25, 2014 at 10:29:36PM +0100, Andres Freund wrote:
> > On 2014-01-25 16:28:09 -0500, Bruce Momjian wrote:
> > > On Tue, Jun 18, 2013 at 11:04:25AM -0700, Kevin Grittner wrote:
> > > > D'Arcy J.M. Cain
> > > >
> > > > > Although, the
On Sat, Jan 25, 2014 at 10:29:36PM +0100, Andres Freund wrote:
> On 2014-01-25 16:28:09 -0500, Bruce Momjian wrote:
> > On Tue, Jun 18, 2013 at 11:04:25AM -0700, Kevin Grittner wrote:
> > > D'Arcy J.M. Cain
> > >
> > > > Although, the more I think about it, the more I think that the comment
> > >
On 2014-01-25 16:28:09 -0500, Bruce Momjian wrote:
> On Tue, Jun 18, 2013 at 11:04:25AM -0700, Kevin Grittner wrote:
> > D'Arcy J.M. Cain
> >
> > > Although, the more I think about it, the more I think that the comment
> > > is both confusing and superfluous. The code itself is much clearer.
> >
On Tue, Jun 18, 2013 at 11:04:25AM -0700, Kevin Grittner wrote:
> D'Arcy J.M. Cain
>
> > Although, the more I think about it, the more I think that the comment
> > is both confusing and superfluous. The code itself is much clearer.
>
> Seriously, if there is any comment there at all, it should
D'Arcy J.M. Cain
> Although, the more I think about it, the more I think that the comment
> is both confusing and superfluous. The code itself is much clearer.
Seriously, if there is any comment there at all, it should be a
succinct explanation for why we didn't do this (which passes `make
chec
On Tue, 18 Jun 2013 19:19:40 +0200
Andres Freund wrote:
> The NULL refers to the *meaning* of the function (remember, it's
> called slot_attisnull) . Which is to test whether an attribute is
> null. Not to a C NULL.
Actually, the comment is not for the function. It only describes the
two lines t
On 2013-06-18 13:14:30 -0400, D'Arcy J.M. Cain wrote:
> On Tue, 18 Jun 2013 11:38:45 +0200
> Andres Freund wrote:
> > > How about "check if attnum is out of range according to the tupdesc"
> > > instead?
> >
> > I can't follow. Minus the word 'NULL' - which carries meaning - your
> > suggested co
On Tue, 18 Jun 2013 11:38:45 +0200
Andres Freund wrote:
> > How about "check if attnum is out of range according to the tupdesc"
> > instead?
>
> I can't follow. Minus the word 'NULL' - which carries meaning - your
> suggested comment pretty much is the same as the existing comment
> except that
Andres Freund wrote:
> On 2013-06-18 05:21:15 -0400, D'Arcy J.M. Cain wrote:
>> On Tue, 18 Jun 2013 11:01:28 +0200
>> Andres Freund wrote:
>> > > /*
>> > > * return true if attnum is out of range according to the tupdesc
>> > > */
>> > > if (attnum > tupleDesc->natts)
>> > > return true;
>> >
>
On 2013-06-18 05:21:15 -0400, D'Arcy J.M. Cain wrote:
> On Tue, 18 Jun 2013 11:01:28 +0200
> Andres Freund wrote:
> > > /*
> > > * return true if attnum is out of range according to the tupdesc
> > > */
> > > if (attnum > tupleDesc->natts)
> > > return true;
> >
> > I think the comment is more m
On Tue, 18 Jun 2013 11:01:28 +0200
Andres Freund wrote:
> > /*
> > * return true if attnum is out of range according to the tupdesc
> > */
> > if (attnum > tupleDesc->natts)
> > return true;
>
> I think the comment is more meaningfull before the change since it
> tells us how nonexisting columns
On Tue, Jun 18, 2013 at 6:01 PM, Andres Freund wrote:
> Hi,
>
> On 2013-06-18 17:56:34 +0900, Amit Langote wrote:
>> Should it be: "return true if attnum is out of range according to the
>> tupdesc" instead of "return NULL if attnum is out of range according
>> to the tupdesc" at src/backend/acces
Hi,
On 2013-06-18 17:56:34 +0900, Amit Langote wrote:
> Should it be: "return true if attnum is out of range according to the
> tupdesc" instead of "return NULL if attnum is out of range according
> to the tupdesc" at src/backend/access/common/heaptuple.c: 1345
>
> /*
> * return true if attnum i
Hi,
Should it be: "return true if attnum is out of range according to the
tupdesc" instead of "return NULL if attnum is out of range according
to the tupdesc" at src/backend/access/common/heaptuple.c: 1345
/*
* return true if attnum is out of range according to the tupdesc
*/
if (attnum > tupleD
28 matches
Mail list logo
