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

lint: apply isort to standardize imports #279

Merged
merged 4 commits into from
Jan 24, 2025
Merged

lint: apply isort to standardize imports #279

merged 4 commits into from
Jan 24, 2025

Conversation

ralphbean
Copy link
Member

No description provided.

webbnh
webbnh previously approved these changes Jan 24, 2025
Copy link
Collaborator

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

LGTM, but I have two questions/thoughts.

First, are you sure that you want to run isort as a separate Tox "environment", rather than running it as an additional command in the lint environment?

Second, I'm a fan of isort, and I recommend a few more options:

known_first_party = ["sync2jira"]  # use separate section for project sources
force_sort_within_sections = true  # don't separate import vs from
order_by_type = false              # sort alphabetic regardless of case

(Notes: Don't include the # comments -- isort cannot read the configuration successfully with them present in the tox.ini file (although, they are OK in a pyproject.toml file -- different rules, I guess). Also, these options will change the sort orders, so you'll need to re-generate the changes.)

@ralphbean ralphbean disabled auto-merge January 24, 2025 20:42
@ralphbean ralphbean merged commit 4324bdc into main Jan 24, 2025
6 checks passed
@ralphbean ralphbean deleted the isort branch January 24, 2025 20:42
Copy link
Collaborator

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

Looks like we collided. Anyway, here are some late-breaking comments, if you're interested.

Comment on lines 20 to 25
from datetime import datetime, timezone

# Python Standard Library Modules
import difflib
import logging
import operator
Copy link
Collaborator

Choose a reason for hiding this comment

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

These imports should all be in one block (i.e., datetime is a Standard Library module), so we should probably just remove the comment at line 22 (and line 29 and line 36) -- now that we have isort, these blocks will be maintained automatically, no there is no need to label them.

Comment on lines +24 to +31
from copy import deepcopy

# Build-In Modules
import logging
import warnings
import traceback
from time import sleep
import requests
from copy import deepcopy
import os
from time import sleep
import traceback
import warnings
Copy link
Collaborator

@webbnh webbnh Jan 24, 2025

Choose a reason for hiding this comment

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

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