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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update inconsistent indentation #962

Merged
merged 2 commits into from
Aug 10, 2021

Conversation

akutz
Copy link
Contributor

@akutz akutz commented Aug 6, 2021

Proposed Commit Message

Update inconsistent indentation

This patch updates some indentation in a comment that prevented an
attempt to run the Black formatter (https://github.com/psf/black)
against the cloud-init codebase:

    $ find cloudinit -name '*.py' -type f | xargs black -l 79 --check 
    ...
    Oh no! 馃挜 馃挃 馃挜
    262 files would be reformatted, 19 files would be left unchanged, 1 file would fail to reformat.

The one file that fails to format is cloudinit/net/__init__.py, and
running the black command directly against that fail reveals more about
the failure:

    $ black -l 79 --check cloudinit/net/__init__.py
    error: cannot format cloudinit/net/__init__.py: INTERNAL ERROR: Black produced code that is not equivalent to the source.  Please report a bug on https://github.com/psf/black/issues.  This diff might be helpful: /var/folders/0c/ffr4wl4d37924s44ctztx_ym00bgms/T/blk_y_s4uilq.log
    Oh no! 馃挜 馃挃 馃挜
    1 file would fail to reformat.

The highlighted log file contains the abstract syntax tree (AST) that
caused the black command to fail:

    --- src
    +++ dst
    @@ -4558,11 +4558,11 @@
               value=
                 Constant(
                   kind=
                     None,  # NoneType
                   value=
    -                '/* interface name assignment types (sysfs name_assign_type attribute) */\n#define NET_NAME_UNKNOWN\t0\t/* unknown origin (not exposed to user) */\n#define NET_NAME_ENUM\t\t1\t/* enumerated by kernel */\n#define NET_NAME_PREDICTABLE\t2\t/* predictably named by the kernel */\n#define NET_NAME_USER\t\t3\t/* provided by user-space */\n#define NET_NAME_RENAMED\t4\t/* renamed by user-space */',  # str
    +                '/* interface name assignment types (sysfs name_assign_type attribute) */\n#define NET_NAME_UNKNOWN    0       /* unknown origin (not exposed to user) */\n#define NET_NAME_ENUM               1       /* enumerated by kernel */\n#define NET_NAME_PREDICTABLE        2       /* predictably named by the kernel */\n#define NET_NAME_USER               3       /* provided by user-space */\n#define NET_NAME_RENAMED    4       /* renamed by user-space */',  # str
                 )  # /Constant
             )  # /Expr
             Assign(
               targets=
                 Name(

With this fix in place, the black command can successfully parse the
file into AST and back again:

    $ black -l 79 --check cloudinit/net/__init__.py                
    would reformat cloudinit/net/__init__.py
    Oh no! 馃挜 馃挃 馃挜
    1 file would be reformatted.

Normally this patch would be part of such an overall effort, but since
this is the only location that interrupted running the black command,
this author felt it was worth addressing this discrepancy sooner than
later in the case there is subsequent desire to use a standard format
tool such as black.

Additional Context

NA

Test Steps

The updated section was in a docstring, but nevertheless make unittest was executed to ensure all rapid testing still passed.

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly

This patch updates some indentation in a comment that prevented an
attempt to run the Black formatter (https://github.com/psf/black)
against the cloud-init codebase:

    $ find cloudinit -name '*.py' -type f | xargs black -l 79 --check
    ...
    Oh no! 馃挜 馃挃 馃挜
    262 files would be reformatted, 19 files would be left unchanged, 1 file would fail to reformat.

The one file that fails to format is cloudinit/net/__init__.py, and
running the black command directly against that fail reveals more about
the failure:

    $ black -l 79 --check cloudinit/net/__init__.py
    error: cannot format cloudinit/net/__init__.py: INTERNAL ERROR: Black produced code that is not equivalent to the source.  Please report a bug on https://github.com/psf/black/issues.  This diff might be helpful: /var/folders/0c/ffr4wl4d37924s44ctztx_ym00bgms/T/blk_y_s4uilq.log
    Oh no! 馃挜 馃挃 馃挜
    1 file would fail to reformat.

The highlighted log file contains the abstract syntax tree (AST) that
caused the black command to fail:

    --- src
    +++ dst
    @@ -4558,11 +4558,11 @@
               value=
                 Constant(
                   kind=
                     None,  # NoneType
                   value=
    -                '/* interface name assignment types (sysfs name_assign_type attribute) */\n#define NET_NAME_UNKNOWN\t0\t/* unknown origin (not exposed to user) */\n#define NET_NAME_ENUM\t\t1\t/* enumerated by kernel */\n#define NET_NAME_PREDICTABLE\t2\t/* predictably named by the kernel */\n#define NET_NAME_USER\t\t3\t/* provided by user-space */\n#define NET_NAME_RENAMED\t4\t/* renamed by user-space */',  # str
    +                '/* interface name assignment types (sysfs name_assign_type attribute) */\n#define NET_NAME_UNKNOWN    0       /* unknown origin (not exposed to user) */\n#define NET_NAME_ENUM               1       /* enumerated by kernel */\n#define NET_NAME_PREDICTABLE        2       /* predictably named by the kernel */\n#define NET_NAME_USER               3       /* provided by user-space */\n#define NET_NAME_RENAMED    4       /* renamed by user-space */',  # str
                 )  # /Constant
             )  # /Expr
             Assign(
               targets=
                 Name(

With this fix in place, the black command can successfully parse the
file into AST and back again:

    $ black -l 79 --check cloudinit/net/__init__.py
    would reformat cloudinit/net/__init__.py
    Oh no! 馃挜 馃挃 馃挜
    1 file would be reformatted.

Normally this patch would be part of such an overall effort, but since
this is the only location that interrupted running the black command,
this author felt it was worth addressing this discrepancy sooner than
later in the case there is subsequent desire to use a standard format
tool such as black.
@akutz
Copy link
Contributor Author

akutz commented Aug 9, 2021

Rebased onto main

Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is somewhat amusing.

Just FYI, for now we still enforce the 79-char line endings, so black may not always produce code that fits our coding standard.

@akutz
Copy link
Contributor Author

akutz commented Aug 9, 2021

Hi @TheRealFalcon,

Just FYI, for now we still enforce the 79-char line endings, so black may not always produce code that fits our coding standard.

That's why I used -l 79 to tell Black to use 79 characters :)

@TheRealFalcon
Copy link
Member

@akutz , the CLA file change is still waiting in your other PR. I'm going to wait until that one merges first. No point in further rebases...I can do that once I'm ready to merge.

@akutz
Copy link
Contributor Author

akutz commented Aug 9, 2021

Hi @TheRealFalcon,

Ack. I'll stop rebasing the other PR as well then. I think it should be ready for Chad's approval once he takes a look. At least in so far that I addressed all of his feedback. Fingers crossed :)

@TheRealFalcon TheRealFalcon merged commit c62cb3a into canonical:main Aug 10, 2021
TheRealFalcon pushed a commit to TheRealFalcon/cloud-init that referenced this pull request Aug 10, 2021
This patch updates some indentation in a comment that prevented an
attempt to run the Black formatter (https://github.com/psf/black)
against the cloud-init codebase:

    $ find cloudinit -name '*.py' -type f | xargs black -l 79 --check
    ...
    Oh no! 馃挜 馃挃 馃挜
    262 files would be reformatted, 19 files would be left unchanged, 1 file would fail to reformat.

The one file that fails to format is cloudinit/net/__init__.py.

With this fix in place, the black command can successfully parse the
file into AST and back again:

    $ black -l 79 --check cloudinit/net/__init__.py
    would reformat cloudinit/net/__init__.py
    Oh no! 馃挜 馃挃 馃挜
    1 file would be reformatted.

Normally this patch would be part of such an overall effort, but since
this is the only location that interrupted running the black command,
this author felt it was worth addressing this discrepancy sooner than
later in the case there is subsequent desire to use a standard format
tool such as black.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants