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

Remove all python shebang instances from package #545

Merged
merged 1 commit into from
Mar 28, 2023

Conversation

ckilcoin
Copy link

@ckilcoin ckilcoin commented Mar 21, 2023

Fixes: #544 and #498

Other instances are in diff files, actual generated python code, or unrelated to python shebang instances

Followup from discussion / work in #544 and #498

Lines can be removed since Python2 support has been deprecated and all modules work with Python3 and do not need to specify which python instance to use.

Please verify and check that the pull request fulfils the following requirements:

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

Followup from work in secure-systems-lab#544 and secure-systems-lab#498
Other instances are in diff files, actual generated python code,
or unrelated to python shebang instances
@lukpueh
Copy link
Member

lukpueh commented Mar 22, 2023

Thanks for the patch, @ckilcoin! I'd appreciate a short comment on why it's okay to remove all these shebang lines.

Copy link
Member

@lukpueh lukpueh left a comment

Choose a reason for hiding this comment

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

Lines can be removed since Python2 support has been deprecated and all modules work with Python3 and do not need to specify which python instance to use.

Thanks for adding the comment. I was actually looking for something like... the relevant modules are generally imported from other modules and not run as script and the tests may very well be executed by specifying the interpreter explicitly, if executed individually...

@lukpueh lukpueh merged commit aa3f606 into secure-systems-lab:main Mar 28, 2023
@ckilcoin ckilcoin deleted the colm-development branch March 30, 2023 02:53
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.

2 participants