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

isort fixes #1390

Merged
merged 1 commit into from
Sep 10, 2022
Merged

isort fixes #1390

merged 1 commit into from
Sep 10, 2022

Conversation

alanzhu0
Copy link
Member

@alanzhu0 alanzhu0 commented Sep 9, 2022

Proposed changes

  • isort recursive flag fix
  • make isort happy in a few files (sorry I missed this before since linting wasn't getting to isort because of pylint)

@alanzhu0 alanzhu0 requested review from a team and shahsalonik as code owners September 9, 2022 22:21
@coveralls
Copy link

coveralls commented Sep 9, 2022

Coverage Status

Coverage remained the same at 82.151% when pulling c4305ec on alanzhu0:fixes into da8a960 on tjcsl:dev.

Copy link
Contributor

@shahsalonik shahsalonik left a comment

Choose a reason for hiding this comment

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

This looks good, but can you squash your commits? After that, I'll merge it :)

@alanzhu0
Copy link
Member Author

Oh so actually these are supposed to be two different commits since they deal with different parts of the codebase - is that okay?

@shahsalonik
Copy link
Contributor

Yeah, so even though they deal with two different things, you should still squash them.

@alanzhu0
Copy link
Member Author

Oh okay I'll do that - I'm not on my computer right now but I'll get it done by tomorrow!

@shahsalonik
Copy link
Contributor

Yup, no big deal. Whenever you have time is good!

isort --recursive flag is not needed because it
is default in v5.0.0
sort imports correctly in cslapps module
@shahsalonik shahsalonik merged commit 1d6fa91 into tjcsl:dev Sep 10, 2022
@alanzhu0 alanzhu0 deleted the fixes branch October 2, 2022 17:17
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.

3 participants