Skip to content

Commit

Permalink
Add isort third party detection from requirements.txt
Browse files Browse the repository at this point in the history
The current setup used in the OCA for isort is done by using 2
pre-commit hooks:

1. seed-isort-config: find third-party libs and updates the
'.isort.cfg' file's "known_third_party" option with the list
2. isort: runs isort, which will use the list of third party
libs provided by 'seed-isort-config' in the "known_third_party" option.

This is an issue [0] with the workflow adopted by many contributors of
the OCA: as the pull requests may need some time to be merged, we use
temporary branches where we aggregate the various PRs.

If several pull requests add a third party library, we'll have a
conflict, and when the process of merging these pull requests is
automated (e.g. with git-aggregator [1]), it becomes tedious.

The list of libs in the "known_third_party" variable is not even
editable manually, as "seed-isort-config" overwrite the value on every
commit: it doesn't really make sense to actually store this.

As pointed out by @sbidoul [2], isort mentions in their changelog:

> Support for using requirements files to auto determine third-paty
> section if pipreqs & requirementslib are installed.

Which is the goal of this change, details:

* Remove 'seed-isort-config' from pre-commit as the list of libs will be
  provided by requirements.txt
* Replace the isort pre-commit repo by the official repo (the mirror says
  it is been deprecated in favor of the official repo)
* Adds pipreqs and pip-api in additional dependencies to activate
  isort's feature to read requirements.txt [3]
* Adds types: [python] in the pre-commit config: the mirror had it and
  the official repo doesn't. Without it, some files such as .pot are
  modified (different EOF). (it can be removed after the next isort
  release)
* Add an union merge driver on 'requirements.txt' to resolve conflicts
  on this file (on conflicts, both lines are kept, which works in most
  cases on this file, in the rare case it's an issue because we have
  2 requirements for different versions, we'll have to fix manually)

Note: it is now important to have the Python libraries declared in
"requirements.txt" to have them ordered in the proper place. At some
point, this file could be automatically generated: [4].

Alternatively, we could have tried 'reorder-python-imports' in place of 'isort' [5].

This setup has been tested successfully on OCA/stock-logistics-warehouse#828

[0] OCA#625
[1] https://github.com/acsone/git-aggregator
[2] OCA#625 (comment)
[3] https://github.com/timothycrosley/isort/blob/500bafabbd51a6005c11a00c4738a2438990e48a/pyproject.toml#L42
[4] OCA/oca-github-bot#98
[5] https://github.com/asottile/reorder_python_imports

Closes OCA#625
  • Loading branch information
guewen committed Jan 27, 2020
1 parent 194e87a commit 3f4d743
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 7 deletions.
2 changes: 2 additions & 0 deletions sample_files/pre-commit-13.0/.gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Limit conflicts in PRs by automatically merging lines of both sides
requirements.txt merge=union
1 change: 0 additions & 1 deletion sample_files/pre-commit-13.0/.isort.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,3 @@ line_length=88
known_odoo=odoo
known_odoo_addons=odoo.addons
sections=FUTURE,STDLIB,THIRDPARTY,ODOO,ODOO_ADDONS,FIRSTPARTY,LOCALFOLDER
known_third_party=
10 changes: 4 additions & 6 deletions sample_files/pre-commit-13.0/.pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,11 @@ repos:
rev: v1.26.2
hooks:
- id: pyupgrade
- repo: https://github.com/asottile/seed-isort-config
rev: v1.9.4
hooks:
- id: seed-isort-config
- repo: https://github.com/pre-commit/mirrors-isort
rev: v4.3.21
- repo: https://github.com/timothycrosley/isort
rev: 4.3.21-2
hooks:
- id: isort
name: isort except __init__.py
exclude: /__init__\.py$
'types': [python]
additional_dependencies: [pipreqs, pip-api]
2 changes: 2 additions & 0 deletions sample_files/pre-commit-13.0/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Add the python requirements here.
# Note: the libs in this file will be considered as "third-party libs" by isort.

0 comments on commit 3f4d743

Please sign in to comment.