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

Fixing CircleCI #2444

Merged
merged 3 commits into from Sep 4, 2019

Conversation

@heavelock
Copy link
Contributor

commented Sep 3, 2019

I noticed that CircleCi is not working due to lack of conda on offician conda image. Maybe it's because commands were not using full path to conda?

heavelock added 3 commits Sep 3, 2019
@heavelock

This comment has been minimized.

Copy link
Contributor Author

commented Sep 3, 2019

Okay, linting works. In short, I added fullpaths to both conda and flake8 executables. I also removed whole part of environment creation, I think it is a bit redundant since we are anyway spinning up a new image every for each test.

@heavelock heavelock requested a review from megies Sep 3, 2019
@d-chambers

This comment has been minimized.

Copy link
Member

commented Sep 3, 2019

Would ignoring W504 for now make sense? There was some talk of using black in the future which would fix all of those types of issues. Over than W504, it looks like there are only a handful of formatting errors to deal with.

@heavelock

This comment has been minimized.

Copy link
Contributor Author

commented Sep 3, 2019

I'm also all for black but for now I just wanted to make the CircleCI running again :)

@d-chambers

This comment has been minimized.

Copy link
Member

commented Sep 3, 2019

Yes, and it looks like it runs now, which is a huge improvement. Do you want to merge this now that the linter runs and deal with the linting issues in a different PR?

@heavelock

This comment has been minimized.

Copy link
Contributor Author

commented Sep 4, 2019

Yup, I think the linting fixes should be separated out, it might get messy otherwise. And people doing PRs in meantime might fix their own mess.

@heavelock

This comment has been minimized.

Copy link
Contributor Author

commented Sep 4, 2019

And to be honest, looking at how many of E504 there are and what that error means, I think it would be good to ignore them. It will be super useless work to fix that since we hope to move to black.

@megies

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

+1 for ignoring e/w504

@megies
megies approved these changes Sep 4, 2019
@megies megies merged commit a5c9731 into obspy:master Sep 4, 2019
0 of 4 checks passed
0 of 4 checks passed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
ci/circleci Your tests failed on CircleCI
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
docker-testbot docker testbot results not available yet
@megies

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

Thanks @heavelock

@QuLogic QuLogic added this to the 1.2.0 milestone Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.