Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup a few minor things #61075

Closed
serhiy-storchaka opened this issue Jan 5, 2013 · 9 comments
Closed

Cleanup a few minor things #61075

serhiy-storchaka opened this issue Jan 5, 2013 · 9 comments
Labels
stdlib Python modules in the Lib dir tests Tests in the Lib/test dir type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 16871
Nosy @jcea, @josiahcarlson, @orsenthil, @giampaolo, @mitsuhiko, @ezio-melotti, @cjerdonek, @serhiy-storchaka
Files
  • minor_things_parens-2.7.diff
  • minor_things_parens-3.2.diff
  • minor_things_parens-3.3.diff
  • minor_things_parens-3.4.diff
  • minor_things_spaces-3.2.diff
  • minor_things_spaces-2.7.diff
  • minor_things_spaces-3.3.diff
  • minor_things_spaces-3.4.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2013-01-08.08:11:57.529>
    created_at = <Date 2013-01-05.16:08:57.735>
    labels = ['tests', 'type-feature', 'library']
    title = 'Cleanup a few minor things'
    updated_at = <Date 2013-01-08.11:35:08.760>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2013-01-08.11:35:08.760>
    actor = 'giampaolo.rodola'
    assignee = 'none'
    closed = True
    closed_date = <Date 2013-01-08.08:11:57.529>
    closer = 'serhiy.storchaka'
    components = ['Library (Lib)', 'Tests']
    creation = <Date 2013-01-05.16:08:57.735>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['28576', '28577', '28578', '28579', '28580', '28581', '28582', '28583']
    hgrepos = []
    issue_num = 16871
    keywords = ['patch']
    message_count = 9.0
    messages = ['179128', '179130', '179133', '179138', '179139', '179148', '179311', '179320', '179336']
    nosy_count = 10.0
    nosy_names = ['jcea', 'josiahcarlson', 'orsenthil', 'giampaolo.rodola', 'gpolo', 'stutzbach', 'aronacher', 'ezio.melotti', 'chris.jerdonek', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'rejected'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue16871'
    versions = ['Python 3.4']

    @serhiy-storchaka
    Copy link
    Member Author

    Here is a set of patches which clean a few minor things: add spaces after if/while/return/etc, remove spaces after class/function name, remove redundant parens after if/while/return/etc. One set contains only space changes, another set contains parens changes.

    @serhiy-storchaka serhiy-storchaka added stdlib Python modules in the Lib dir tests Tests in the Lib/test dir labels Jan 5, 2013
    @orsenthil
    Copy link
    Member

    Looks good in most of the places, but at some places the parenthesis are of
    course helpful for cohesiveness, and this can be quite subjective. With
    these patches, are you in general removing all instances of parenthesis
    when it is not required or also considering for places where parens may
    help while reading the code?

    +1 for the space changes patches.

    On Sat, Jan 5, 2013 at 8:14 AM, Serhiy Storchaka <report@bugs.python.org>wrote:

    Changes by Serhiy Storchaka <storchaka@gmail.com>:

    ----------
    keywords: +patch
    Added file: http://bugs.python.org/file28576/minor_things_parens-2.7.diff
    Added file: http://bugs.python.org/file28577/minor_things_parens-3.2.diff
    Added file: http://bugs.python.org/file28578/minor_things_parens-3.3.diff
    Added file: http://bugs.python.org/file28579/minor_things_parens-3.4.diff
    Added file: http://bugs.python.org/file28580/minor_things_spaces-3.2.diff
    Added file: http://bugs.python.org/file28581/minor_things_spaces-2.7.diff
    Added file: http://bugs.python.org/file28582/minor_things_spaces-3.3.diff
    Added file: http://bugs.python.org/file28583/minor_things_spaces-3.4.diff


    Python tracker <report@bugs.python.org>
    <http://bugs.python.org/issue16871\>



    Python-bugs-list mailing list
    Unsubscribe:
    http://mail.python.org/mailman/options/python-bugs-list/senthil%40uthcode.com

    @serhiy-storchaka
    Copy link
    Member Author

    For me none of these parens help while reading the code (but this is quite subjective). I not object if only a part of these changes will be applied. Review the patches and point what changes should be applied and what changes should not.

    @jcea
    Copy link
    Member

    jcea commented Jan 5, 2013

    How about innecessary code churn?

    @jcea
    Copy link
    Member

    jcea commented Jan 5, 2013

    I elaborate: bpo-15580. An example of many.

    @cjerdonek
    Copy link
    Member

    Here is another recent comment from Georg on this topic:

    "And please don't commit cosmetic/"cleanup" changes to bugfix branches in the future."

    (from http://bugs.python.org/issue16793#msg178372 )

    @ezio-melotti
    Copy link
    Member

    I'm -1 on this, however parts of the patches could be extracted and applied on individual modules in the 'default' branch only. This could be done by the modules' maintainers if they think it's OK.

    Note that while it's generally not OK doing this kind of refactoring on the code in maintenance branches, it's usually acceptable doing so for the documentation.

    @ezio-melotti ezio-melotti added the type-feature A feature request or enhancement label Jan 8, 2013
    @serhiy-storchaka
    Copy link
    Member Author

    Agreed. Maintainers of asynchat, tkinter.tix, optparse and difflib must pay attention to this.

    @giampaolo
    Copy link
    Contributor

    Ok for asyncore/asynchat in 3.4 branch.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir tests Tests in the Lib/test dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants