isort / isort-action

Github Action to run isort
MIT License
22 stars 14 forks source link

add action input to configure working dir #51

Open chenguo opened 2 years ago

chenguo commented 2 years ago

I have a project with a couple sub-packages that have their own requirement.txts, i.e.:

my-proj/
    sub-proj-foo/
        requirements.txt
    sub-proj-bar/
        requirements.txt

running the isort action doesn't pick up the third party deps, since isort is always run from the top-level root.

I'd like to be able to configure two separate isort invocations into my workflow, where I can call isort inside of sub-proj-foo and then separately inside of sub-proj-bar. Is this something you'd consider supporting?

jamescurtin commented 2 years ago

Have you tried using the requirementsFiles and sortPaths options to specify which requirements should be installed & then restrict import sorting to the relevant subdirectory? e.g.

name: Run isort
on:
  - push

jobs:
  lint-foo:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - uses: isort/isort-action@master
        with:
            requirementsFiles: "my-proj/sub-proj-foo/requirements.txt"
            sortPaths: "my-proj/sub-proj-foo"
  lint-bar:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - uses: isort/isort-action@master
        with:
            requirementsFiles: "my-proj/sub-proj-bar/requirements.txt"
            sortPaths: "my-proj/sub-proj-bar"
chenguo commented 2 years ago

Ah I glossed over that. That works for some of our packages, others (that need to be published to an internal registry) use a setup.py, so the pip install -r "$file" in bin/install_packages won't quite work. We also run some other steps in this workflow where it'd be convenient to not have the pip install command be bundled with a linting step, but rather done separately with some auth handling for our internal registry. Or is there something else for running, say, a pip install . .[test]?

chenguo commented 2 years ago

did a quick test, I loaded up a virtualenv and did the equivalent of:

(my-venv) $ cd my-proj
(my-venv) $ isort --check --diff --profile=black .
(my-venv) $ cd sub-proj-foo
(my-venv) $ isort --check --diff --profile=black .

and got different results. Keeping the venv constant should basically have the same effect as running bin/install_packages $PYTHON_VERSION sub-proj-foo/requirements.txt right? So then maybe isort itself is finicky about the path?

jamescurtin commented 2 years ago

We also run some other steps in this workflow where it'd be convenient to not have the pip install command be bundled with a linting step.

Should be totally possible with the current setup. Just exclude the requirementsFiles input and perform whatever setup is required in an earlier step. e.g.

...
jobs:
  lint-foo:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - name: Install dependencies
        run: echo "Installed the required dependencies here..."
      - uses: isort/isort-action@master
        with:
            sortPaths: "my-proj/sub-proj-foo"

... and got different results ... So then maybe isort itself is finicky about the path?

Out of curiosity, what were the differences? It would be easier to say definitively if there was a repro you could share.

Note that if each of the subdirectories has its own config file with different isort settings, the plugin would currently require you to override the default for the configuration input. (Current default is --check-only --diff, and you'd need to update it to something like --check-only --diff --settings-file=my-proj/sub-proj-foo/.isort.cfg)

An alternative would be to add support for another option named something like workingDirectory that would update the working directory of the step & all commands would be run from that relative path. (Which would avoid the need to leak config into the plugin via the configuration input)