Skip to content
This repository was archived by the owner on Mar 13, 2022. It is now read-only.
This repository was archived by the owner on Mar 13, 2022. It is now read-only.

Boilerplate includes un-necessary shebang line #154

Closed

Description

@oz123

Currently the following modules contain this line:

watch/watch.py:#!/usr/bin/env python
watch/__init__.py:#!/usr/bin/env python
watch/watch_test.py:#!/usr/bin/env python
config/incluster_config_test.py:#!/usr/bin/env python
config/incluster_config.py:#!/usr/bin/env python
config/dateutil.py:#!/usr/bin/env python
config/kube_config_test.py:#!/usr/bin/env python
config/exec_provider.py:#!/usr/bin/env python
config/kube_config.py:#!/usr/bin/env python
config/__init__.py:#!/usr/bin/env python
config/config_exception.py:#!/usr/bin/env python
config/exec_provider_test.py:#!/usr/bin/env python
config/dateutil_test.py:#!/usr/bin/env python
stream/stream.py:#!/usr/bin/env python
stream/ws_client.py:#!/usr/bin/env python
stream/ws_client_test.py:#!/usr/bin/env python
stream/__init__.py:#!/usr/bin/env python

We should remove the shebang line from hack/boilerplate/boilerplate.py.txt and then remove it from the modules which aren't expected to run as scripts.

Activity

  1. micw523 commented on Aug 15, 2019

    @micw523
    Contributor

    See discussion in #107 - this has been discussed

  2. oz123 commented on Aug 15, 2019

    @oz123
    ContributorAuthor

    Yes, I see that this was discussed. And I also see that you made this comment in the past:

    I don’t think the python shebang should appear in the non-executable files. This applies to most files you added the shebang.

    What is correct for the main repository isn't blindly correct to a python software. The main repository contains Python scripts which are predestined to be run by CI jobs and developers. For this case it is easier to type ./hack/script-name.py instead of explicitly calling python before the script name. The main (k/k) repository contains:

    $ cd ~/Software/kubernetes
    $ find . -type f -name "*.py"
    ./hack/update_owners.py
    ./hack/boilerplate/test/fail.py
    ./hack/boilerplate/test/pass.py
    ./hack/boilerplate/boilerplate_test.py
    ./hack/boilerplate/boilerplate.py
    ./hack/verify-flags-underscore.py
    ./hack/verify-publishing-bot.py
    ./translations/extract.py
    

    For this repository there no point adding the shebang in Python modules and packages. This looks bad and conveys bad code smell (which might decrease usage in favor for other clients and eventually distract potential contributions).

    IMHO these should be removed.

  3. scottilee commented on Aug 18, 2019

    @scottilee

    +1 I agree with @oz123 on this.

  4. yliaog commented on Aug 20, 2019

    @yliaog
    Contributor

    /cc

  5. oz123 commented on Aug 20, 2019

    @oz123
    ContributorAuthor

    So, there seems to be agreement on this here, I will prepare a PR for this.

  6. yliaog commented on Aug 20, 2019

    @yliaog
    Contributor

    thanks @oz123 please file a pr for this

  7. oz123 commented on Aug 25, 2019

    @oz123
    ContributorAuthor

    @yliaog I created #155 for this, please see if this is OK.

  8. roycaihw commented on Oct 1, 2019

    @roycaihw
    Member

    fixed in #155

    /close

  9. k8s-ci-robot commented on Oct 1, 2019

    @k8s-ci-robot
    Contributor

    @roycaihw: Closing this issue.

    Details

    In response to this:

    fixed in #155

    /close

    Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions