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.

Sign up for GitHub

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

Jump to bottom

Block absolute imports for local imports #1435

Closed
leoporoli opened this issue Sep 29, 2023 · 0 comments · Fixed by #1446
Closed

Block absolute imports for local imports #1435

leoporoli opened this issue Sep 29, 2023 · 0 comments · Fixed by #1446
Assignees
@leoporoli
Labels
feature request painful Painful bug

Comments

@leoporoli
Copy link
Contributor

leoporoli commented Sep 29, 2023 •
edited

Background & motivation

Right now we allow absolute imports for specifying local imports (imports within a package). This is a legacy from the days before we had relative imports, and it’s causing some of the painpoints above.

We should instead ban absolute imports within a package, and force relative imports (either via a leading . or from the package root using / )

from: https://www.notion.so/kurtosistech/2023-09-Forked-Packages-Cleanup-d8bc4f5165ae4b039c07cdad56892459

Desired behavior

This has three benefits:

  • Users don’t have the option to get confused about which syntax they should use
  • Eliminate the “accidentally pull from Github when I didn’t want to” issue
  • Make it clearer at a glance which imports are imported from outside the package, and which imports are imported from inside

How important is this to you?

Painful; the lack of this feature makes using Kurtosis frictionful.
@github-actions github-actions bot added the painful Painful bug label Sep 29, 2023
@leoporoli leoporoli self-assigned this Sep 29, 2023
@leoporoli leoporoli mentioned this issue Sep 29, 2023
Merged
leoporoli added a commit that referenced this issue Oct 3, 2023
@leoporoli @mieubrisse
@galenmarchetti @h4ck3rk3y @kurtosisbot @adschwartz @Peeeekay
feat!: block 'local absolute locators', users should replace' local a…
27ded02 
…bsolute locators' with 'relative locators' (#1446)

## Description:
block 'local absolute locators', users should replace' local absolute
locators' with 'relative locators'

## Is this change user facing?
YES

## References (if applicable):
This should be merged after
#1427
Fix #1435

---------

Co-authored-by: Kevin Today <thetallmonkey@gmail.com>
Co-authored-by: Galen Marchetti <galenmarchetti@gmail.com>
Co-authored-by: Gyanendra Mishra <anomaly.the@gmail.com>
Co-authored-by: kurtosisbot <89932784+kurtosisbot@users.noreply.github.com>
Co-authored-by: kurtosisbot <kurtosisbot@users.noreply.github.com>
Co-authored-by: Anders Schwartz <adschwartz@users.noreply.github.com>
Co-authored-by: Peeeekay <15133250+Peeeekay@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@leoporoli leoporoli

Labels
feature request painful Painful bug
Projects
None yet
Milestone
No milestone
1 participant
@leoporoli