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

Replaced absolute paths to the components with relative ones. #254

Closed
wants to merge 1 commit into from

Conversation

KOLANICH
Copy link
Contributor

@KOLANICH KOLANICH commented Jul 9, 2020

Please fill in the fields below to submit a pull request. The more information
that is provided, the better.

Fixes: #254

Description of the changes being introduced by the pull request:
Importing stuff from an own package by its absolute name is harmful. It causes problems with forks using other names. So I have replaced absolute imports with relative ones.

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

@KOLANICH KOLANICH force-pushed the fix_import branch 2 times, most recently from 0381602 to feba5bc Compare July 9, 2020 12:36
@coveralls
Copy link

coveralls commented Jul 9, 2020

Coverage Status

Coverage decreased (-98.9%) to 0.0% when pulling bdccf48 on KOLANICH:fix_import into c94fcf7 on secure-systems-lab:master.

@KOLANICH KOLANICH force-pushed the fix_import branch 11 times, most recently from 453c024 to 59ce9a7 Compare July 13, 2020 16:38
@joshuagl
Copy link
Collaborator

Whilst I appreciate the suggestion, I'm not convinced this is a change we want to take in securesystemslib. PEP 8 recommends against this style of import unless necessary, and I'm not convinced it is necessary.

Are you aware of a fork for which this import style is causing problems? Can we encourage them to work with us, rather than forking?

@KOLANICH
Copy link
Contributor Author

I don't know any fork it really causes problems to (I haven't searched for any forks), but using absolute imports are making issues from thin air. It not only creates problems for forks, it also creates problems for us. Code using absolute imports is harder to refactor.

@KOLANICH KOLANICH force-pushed the fix_import branch 3 times, most recently from 9b6d030 to b5a6497 Compare August 5, 2020 10:23
@joshuagl
Copy link
Collaborator

Thank you for the submission! Whilst we are very grateful for the contribution we have decided not to merge this PR. We like following the ecosystem norms as defined in PEP 8.

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

3 participants