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

Check isort in CI #913

Merged
merged 2 commits into from
Apr 24, 2020
Merged

Check isort in CI #913

merged 2 commits into from
Apr 24, 2020

Conversation

JayH5
Copy link
Member

@JayH5 JayH5 commented Apr 23, 2020

No description provided.

Copy link
Member

@tomchristie tomchristie left a comment

Choose a reason for hiding this comment

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

Thanks!

Cross checked this against scripts/lint, and it's the same behaviour, soon first glance it makes sense to have this in.

However I'd previous dropped it in 1f7d597#diff-bd0b996d66f03a10364809b532be8f0f - it would have been helpful if my commit was a proper PR with a description attached, but I think it's sometimes the case that being strict on the check there was sometimes introducing issues, so I'd left it in scripts/lint, but removed it as a strict check.

Having said that I'm happy for it to go back in, and we can remove it if there's issues again.

Ideally we'd probably want to reference scripts/lint from scripts/test so that we're not duplicating the behaviour, but that's an issue for a separate PR to consider.

@JayH5 JayH5 merged commit a42657d into encode:master Apr 24, 2020
@JayH5 JayH5 deleted the isort-ci branch April 24, 2020 09:27
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.

2 participants