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.

Sign up for GitHub

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

Jump to bottom

chore: Clean up imports #27302

Merged
merged 9 commits into from
Sep 2, 2021
Merged

chore: Clean up imports #27302

merged 9 commits into from
Sep 2, 2021

Conversation

ChillarAnand
Copy link
Contributor

@ChillarAnand ChillarAnand commented Sep 1, 2021 •
edited
Loading

Used isort/pycln to clean up the imports & added import checks to CI.

@ChillarAnand ChillarAnand force-pushed the imports branch 2 times, most recently from 78f6e7c to 343f6a5 Compare September 1, 2021 16:21
@coveralls
Copy link

coveralls commented Sep 1, 2021 •
edited
Loading

Coverage Status

Coverage decreased (-0.1%) to 51.492% when pulling fe53fdb on ChillarAnand:imports into a158b82 on frappe:develop.

@ChillarAnand ChillarAnand force-pushed the imports branch 6 times, most recently from 4d01b05 to ff3125e Compare September 2, 2021 03:04
@ChillarAnand ChillarAnand force-pushed the imports branch 3 times, most recently from 84f3dcd to 4e5e8b7 Compare September 2, 2021 06:00
@ankush
Copy link
Member

ankush commented Sep 2, 2021 •
edited
Loading

@gavindsouza @surajshetty3416 we are going ahead with sorted imports using isort. The config that will be used is compatible with black style, so when/if we do add black it will work just fine.

This will likely mess up the history for import lines but I guess it's a reasonable compromise. We can add the final commit in .git-blame-ignore-revs but I doubt its efficacy when lots of lines are moved around.

Config file: https://github.com/frappe/erpnext/blob/fe53fdb750a5d70e9e3698ec18b11dd8e88968e3/pyproject.toml

@ChillarAnand ChillarAnand marked this pull request as ready for review September 2, 2021 07:01
@ChillarAnand ChillarAnand requested a review from ankush as a code owner September 2, 2021 07:01
@ankush ankush added squash Meant to tell reviewers that this PR should be squashed into a single commit while merging. manual-porting-required labels Sep 2, 2021
@ankush ankush merged commit 915b343 into frappe:develop Sep 2, 2021
@ankush ankush mentioned this pull request Sep 2, 2021
Closed
@ChillarAnand ChillarAnand mentioned this pull request Sep 2, 2021
Merged
@ankush ankush mentioned this pull request Sep 9, 2021
Closed
asoral pushed a commit to asoral/erpnext that referenced this pull request Nov 12, 2021
@ChillarAnand
chore: Clean up imports (frappe#27302)
e41e183 
* chore: Added isort to pre-commit config

* chore: Sort imports with isort

* chore: Clean up imports with pycln

* chore: Sort imports with isort

* chore: Fix import issues

* chore: Clean up sider issues

* chore: Remove import errors from flake8 ignore list

* chore: Clean up lint issues
@shariquerik shariquerik mentioned this pull request Nov 24, 2021
Merged
@deepeshgarg007 deepeshgarg007 mentioned this pull request Jul 18, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ankush ankush ankush approved these changes

@gavindsouza gavindsouza Awaiting requested review from gavindsouza

@surajshetty3416 surajshetty3416 Awaiting requested review from surajshetty3416

@deepeshgarg007 deepeshgarg007 Awaiting requested review from deepeshgarg007

@marination marination Awaiting requested review from marination

@nextchamp-saqib nextchamp-saqib Awaiting requested review from nextchamp-saqib

@pateljannat pateljannat Awaiting requested review from pateljannat

@rohitwaghchaure rohitwaghchaure Awaiting requested review from rohitwaghchaure rohitwaghchaure is a code owner

@ruchamahabal ruchamahabal Awaiting requested review from ruchamahabal

Assignees
No one assigned
Labels
manual-porting-required squash Meant to tell reviewers that this PR should be squashed into a single commit while merging.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ChillarAnand @coveralls @ankush