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

SUPP: Add black step to CI #1988

Merged
merged 3 commits into from
Oct 4, 2019
Merged

SUPP: Add black step to CI #1988

merged 3 commits into from
Oct 4, 2019

Conversation

toryhaavik
Copy link
Contributor

Closes #1966

@toryhaavik toryhaavik requested a review from xmnlab October 2, 2019 18:50
Copy link
Contributor

@xmnlab xmnlab left a comment

Choose a reason for hiding this comment

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

thanks @toryhaavik for working on that!
I just added a comment about pip section.

@@ -1,7 +1,7 @@
channels:
- conda-forge
dependencies:
- black
- git+git://github.com/psf/black@40e8b3a231bade22d838858435a33d60a8325306#egg=black # see .pre-commit-config.yaml, we pin to a specific version of black
Copy link
Contributor

Choose a reason for hiding this comment

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

this probably should be in a pip section.

at the end, inside the pip section after - seed-isort-config you can add:

    - git+https://github.com/psf/black.git@40e8b3a231bade22d838858435a33d60a8325306
    - git+https://github.com/timothycrosley/isort.git@18ad293fc9d1852776afe35015a932b68d26fb14

also we need a specific version of isort

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I should have seen the pip section before...

@@ -1,7 +1,7 @@
channels:
- conda-forge
dependencies:
- black
- git+git://github.com/psf/black@40e8b3a231bade22d838858435a33d60a8325306#egg=black # see .pre-commit-config.yaml, we pin to a specific version of black
Copy link
Contributor

Choose a reason for hiding this comment

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

see the previous comment about pip section

@toryhaavik
Copy link
Contributor Author

Ha! the pipeline failed correctly :D. There are some formatting changes that need to be made, i'll add them to this review.

@xmnlab
Copy link
Contributor

xmnlab commented Oct 4, 2019

@toryhaavik that is awesome!

It seems black is not available inside the CI for linux/python 3.5:

/activate.sh: line 1: exec: black: not found

@toryhaavik
Copy link
Contributor Author

Oh yeah, because black doesn't work on py35. I'll have to add a conditional to check version

Copy link
Contributor

@xmnlab xmnlab left a comment

Choose a reason for hiding this comment

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

LGTM! thanks @toryhaavik for working on that!

@xmnlab xmnlab merged commit f5a0a6e into ibis-project:master Oct 4, 2019
costrouc pushed a commit to costrouc/ibis that referenced this pull request Oct 10, 2019
Closes ibis-project#1966
Author: Tory Haavik <tory.haavik@twosigma.com>

Closes ibis-project#1988 from toryhaavik/black and squashes the following commits:

2073b2e [Tory Haavik] Fix variables
e5db7c7 [Tory Haavik] Apply black formatting
b82bc09 [Tory Haavik] SUPP: Add black step to CI to ensure any proposed changes are already blackened
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.

Add CI steps to check that formatting/linting is correct
2 participants