[issue28838] Using consistent naming for arguments of "call" functions (C API)

[R. David Murray]

R. David Murray added the comment:

"Uniformize" isn't really an English word :)

--
nosy: +r.david.murray
title: Uniformize argument names of "call" functions (C API) -> Using 
consistent naming for arguments of "call" functions (C API)

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28838] Using consistent naming for arguments of "call" functions (C API)

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Oh, you have mixed up comments! In headers descriptions are written after 
function declarations. You have moved some comments and now some descriptions 
are placed below the corresponding declaration, and some descriptions are 
placed above. Now it is not clear to what function every comment is related.

You should either revert your changes or change *all* headers by moving *all* 
descriptions above corresponding declarations. That would be very large patch, 
and I doubt it will be approved.

Please don't make such large changes without a review.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28838] Using consistent naming for arguments of "call" functions (C API)

[Roundup Robot]

Roundup Robot added the comment:

New changeset 69c7451c3ec1 by Victor Stinner in branch 'default':
Issue #28838: Fix weird indentation of abstract.h
https://hg.python.org/cpython/rev/69c7451c3ec1

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28838] Using consistent naming for arguments of "call" functions (C API)

[STINNER Victor]

STINNER Victor added the comment:

I pushed another large patch to "fix" the indentation of abstract.h. I would 
prefer to also have this "reindent" change on review, but our reviewing tool 
doesn't support patch series :-(


Attached cleanup.patch reformats deeply the header file to use the same style 
for all comments, use the same style used in other Python headers. IMO it makes 
the header file more consistent with other header files and it makes the file 
more readable.


R. David Murray (r.david.murray):
> Uniformize" isn't really an English word :)

Oh, it's a frenglish word in this case :-) Good to know.


Serhiy Storchaka: "You have moved some comments and now some descriptions are 
placed below the corresponding declaration, and some descriptions are placed 
above. Now it is not clear to what function every comment is related."

It don't think that my change makes abstract.h worse, some comments were far 
from their declaration, and there is was no unique style for comments. It's a 
mess.


Serhiy: "You should either revert your changes or change *all* headers by 
moving *all* descriptions above corresponding declarations."

My plan is to rewrite completely abstact.h, this file annoys me since many 
months :-)


"That would be very large patch, and I doubt it will be approved."

Why not?


"Please don't make such large changes without a review."

You reviewed rename.patch, I only made tiny changes after this patch. As I 
explained, it's hard to work on such large patch.

--
Added file: http://bugs.python.org/file45777/cleanup.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28838] Using consistent naming for arguments of "call" functions (C API)

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

> It don't think that my change makes abstract.h worse, some comments were far
> from their declaration, and there is was no unique style for comments. It's
> a mess.

Seems most of comments that did not follow common style are your comments to 
"fast call" functions. It would be better to fix only that comments.

> "That would be very large patch, and I doubt it will be approved."
> Why not?

Because it breaks "hg annotation" and makes harder researching the history of 
the file. That harms the workflow of other developers. Ask on Python-Dev before 
making such larger and questionable changes.

> You reviewed rename.patch, I only made tiny changes after this patch.

Reviewed patch did not contain committed changes. That changes are not tiny.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28838] Using consistent naming for arguments of "call" functions (C API)

[STINNER Victor]

STINNER Victor added the comment:

Serhiy:
> Seems most of comments that did not follow common style are your comments to  
> "fast call" functions. It would be better to fix only that comments.

I would like to modify abstract.h because its weird style annoy me for years. I 
would like to make the header consistent with all other headers.

I checked unicodeobject.h, code.h, frameobject.h, floatobject.h, etc.: comments 
are always before declaration.

About "fast calls" in abstract.h, sorry, it's my fault. When I added new code, 
I used the same style than all other files. I didn't notice that abstract.h has 
a special style. That's why I would like to make it consistent again.


Serhiy:
> Reviewed patch did not contain committed changes.

You wrote on the review that a comment is misplaced: "The comment is related to 
PyObject_Call() declared 85 lines above."

That's why I took the opportunity of a refactoring change to fix comments.


Serhiy:
> That changes are not tiny.

Between rename.patch and the pushed change e6ad41a3f3bd, I only moved 5 
comments: see attached rename_pushed.patch which is the diff between the two 
patches.

For me, it was obvious that the comment must be before the declaration, since 
it is the style used in all other Python headers. Sorry, I didn't expect that 
you didn't want to change that. I misunderstood your comment on the review.

--
Added file: http://bugs.python.org/file45779/rename_pushed.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28838] Using consistent naming for arguments of "call" functions (C API)

[STINNER Victor]

STINNER Victor added the comment:

cleanup-2.patch: Rebase cleanup.patch.

This patch is the last part of my work to cleanup Include/abstract.h to make it 
more consistent with other header files.

--
Added file: http://bugs.python.org/file45798/cleanup-2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28838] Using consistent naming for arguments of "call" functions (C API)

[Roundup Robot]

Roundup Robot added the comment:

New changeset 654ec6ed3225 by Victor Stinner in branch 'default':
Issue #28838: Cleanup abstract.h
https://hg.python.org/cpython/rev/654ec6ed3225

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28838] Using consistent naming for arguments of "call" functions (C API)

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Victor, it would be worth to discuss this change on Python-Dev.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28838] Using consistent naming for arguments of "call" functions (C API)

[STINNER Victor]

STINNER Victor added the comment:

I pushed cleanup-2.patch (after a minor rebase). Sorry for breaking "hg 
annotation" on Include/abstract.h: I hope that it will be less annoying that 
the giant "replace tabs with space" made by Antoine Pitrou a few years ago 
which modified almost all .c and .h files (my change only changes a single 
file, abstract.h). If you have large pending patch series which modify this 
file, ping me, I will help you to rebase them.

Sorry, I didn't expect this issue to be so painful. I hope that abstract.h is 
now more readable, easier to maintain (no more surprising indentation!), and 
have better comments. I now started to cleanup the documentation of functions 
of the "PyObject_Call() family": see my first issue #28977. I think that we 
should then synchronize .rst doc and .h comments to make them consistent again 
(and maybe update the doc).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28838] Using consistent naming for arguments of "call" functions (C API)

[STINNER Victor]

STINNER Victor added the comment:

Serhiy: "Victor, it would be worth to discuss this change on Python-Dev."

Done: https://mail.python.org/pipermail/python-dev/2016-December/146999.html

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28838] Using consistent naming for arguments of "call" functions (C API)

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thanks Victor.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28838] Using consistent naming for arguments of "call" functions (C API)

[STINNER Victor]

STINNER Victor added the comment:

I consider that the cleanup itself is done, so I close the issue. For further 
enhancements, I will open new issues.

--
resolution:  -> fixed
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28838] Using consistent naming for arguments of "call" functions (C API)

[Roundup Robot]

Roundup Robot added the comment:

New changeset 3ab0a6692e25 by Victor Stinner in branch 'default':
abstract.h: remove long outdated comment
https://hg.python.org/cpython/rev/3ab0a6692e25

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28838] Using consistent naming for arguments of "call" functions (C API)

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Since no one on Python-Dev has objections, I'm fine with this too. I was 
opposed not so much to the changes themselves, as to making large changes to 
one of basic files that could affect everybody without wider discussion. Excuse 
me if I looked unfriendly.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28838] Using consistent naming for arguments of "call" functions (C API)

[STINNER Victor]

STINNER Victor added the comment:

Serhiy Storchaka added the comment:
> Since no one on Python-Dev has objections, I'm fine with this too. I was 
> opposed not so much to the changes themselves, as to making large changes to 
> one of basic files that could affect everybody without wider discussion. 
> Excuse me if I looked unfriendly.

No problem. Well, it's just reformating. It doesn't change the API for
example. It should only impact people maintaining large external
patches on Python 3.6. I'm not aware of any pending patch modifying
abstract.h. And as I wrote, if there is such patch, contact me, I will
help to rebase your change.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com