Skip to content

Comments

Support for conda as a language#1232

Merged
asottile merged 1 commit intopre-commit:masterfrom
xhochy:conda-language
Dec 21, 2019
Merged

Support for conda as a language#1232
asottile merged 1 commit intopre-commit:masterfrom
xhochy:conda-language

Conversation

@xhochy
Copy link
Contributor

@xhochy xhochy commented Dec 2, 2019

Fixes #1204

Example usage

.pre-commit-hooks.yaml:

- id: black-conda
  name: black-conda
  description: "Black: The uncompromising Python code formatter"
  entry: black
  language: conda
  require_serial: true
  types: [python]

environment.yml:

channels:
  - conda-forge
  - defaults
dependencies:
  - black=19.10b0

@xhochy
Copy link
Contributor Author

xhochy commented Dec 2, 2019

This is lacking sufficient tests, I'm unsure what should be tested and am open for suggestions.

@xhochy xhochy force-pushed the conda-language branch 2 times, most recently from 30ed0d1 to 79779be Compare December 2, 2019 16:13
asottile
asottile reviewed Dec 2, 2019
View reviewed changes
asottile
asottile reviewed Dec 2, 2019
View reviewed changes
asottile
asottile reviewed Dec 2, 2019
View reviewed changes
asottile
asottile reviewed Dec 2, 2019
View reviewed changes
@xhochy
Copy link
Contributor Author

xhochy commented Dec 3, 2019

This has now all review comments incorporated and tests are passing except for Windows. There we need to download a lightweight conda (take an exe from https://repo.anaconda.com/pkgs/misc/conda-execs/) to get the tests passing. This needs an modification of the Azure template like https://github.com/asottile/azure-pipeline-templates/blob/f8721c757e218487ca0bb38af0201b2a04e9ed52/job--python-tox.yml#L63-L68 @asottile Should I modify the template to always install a conda.exe on Windows or make this conditional on some parameter?

@asottile
Copy link
Member

asottile commented Dec 3, 2019

there's a pre_test list where you can install whatever you need 👍

for example, here's what it does to install ruby / swift on linux:

pre-commit/azure-pipelines.yml

Lines 41 to 46 in 023b337

pre_test:
- task: UseRubyVersion@0
- bash: |
testing/get-swift.sh
echo '##vso[task.prependpath]/tmp/swift/usr/bin'
displayName: install swift

@xhochy xhochy force-pushed the conda-language branch 10 times, most recently from 5aba042 to f7144ad Compare December 3, 2019 16:09
asottile
asottile reviewed Dec 3, 2019
View reviewed changes
Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

seems good!

@xhochy
Copy link
Contributor Author

xhochy commented Dec 4, 2019

@asottile Adjusted all things and CI is still green. :)

asottile
asottile reviewed Dec 4, 2019
View reviewed changes
pre_commit/languages/conda.py Outdated

@contextlib.contextmanager
def in_env(prefix, language_version):
helpers.assert_version_default('conda', language_version)
Copy link
Member

@asottile asottile Dec 4, 2019

Choose a reason for hiding this comment

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

this should be part of install_environment I believe

Copy link
Contributor Author

@xhochy xhochy Dec 6, 2019

Choose a reason for hiding this comment

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

Should these be removed here then?

Copy link
Member

@asottile asottile Dec 6, 2019

Choose a reason for hiding this comment

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

yes -- you can take precedent from the other languages that use this function

@xhochy
Copy link
Contributor Author

xhochy commented Dec 6, 2019

@asottile Azure is failing with missing files in the template repo but these actually exist. Do you have an idea what is going wrong here?

@asottile
Copy link
Member

asottile commented Dec 6, 2019

@asottile Azure is failing with missing files in the template repo but these actually exist. Do you have an idea what is going wrong here?

huh they must have changed the inclusion rules :/

@asottile
Copy link
Member

asottile commented Dec 6, 2019

I'm trying to fix that in #1234 -- we'll see!

asottile
asottile reviewed Dec 6, 2019
View reviewed changes
@asottile
Copy link
Member

asottile commented Dec 6, 2019

ok if you rebase on master it should fix the CI

asottile
asottile approved these changes Dec 9, 2019
View reviewed changes
Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

looks good! can probably squash the commits (if you want, don't need to though and don't feel obligated to)

I'm going to take this for a spin locally and then probably merge (so probably within a few days)!

thanks again for the work on this 🎉

@asottile asottile merged commit 0ecd50b into pre-commit:master Dec 21, 2019
@xhochy xhochy deleted the conda-language branch January 12, 2020 15:31
@lorenzwalthert lorenzwalthert mentioned this pull request Mar 27, 2020
Closed
@lorenzwalthert lorenzwalthert mentioned this pull request Nov 1, 2020
Open
@lorenzwalthert lorenzwalthert mentioned this pull request Feb 15, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asottile asottile asottile approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support for conda as a language

2 participants

@xhochy @asottile