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

Add option to select specific fixer to run #333

Closed
wants to merge 2 commits into from
Closed

Add option to select specific fixer to run #333

wants to merge 2 commits into from

Conversation

davidszotten
Copy link

Add --fixer foo option (can be repeated) to only run specific fixers. Fixes #278

adamchainz
adamchainz requested changes Mar 7, 2023
View reviewed changes
Copy link
Owner

@adamchainz adamchainz left a comment

Choose a reason for hiding this comment

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

please add changelog note, and update the readme to include fixer names

i'll review this properly later

src/django_upgrade/data.py Outdated
ast_funcs: ASTCallbackMapping = defaultdict(list)
for fixer in FIXERS:
# TODO: is this where we want to do the name extraction?
Copy link
Owner

Choose a reason for hiding this comment

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

no

tests/test_main.py Outdated
Comment on lines 126 to 131
if changed:
assert result == 1
assert out == "from django.core.paginator import Paginator\n"
else:
assert result == 0
assert out == input_
Copy link
Owner

Choose a reason for hiding this comment

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

I don't like conditional tests, it makes the test code too complicated. can we instead have two test functions, one, for the "changes expected case", one for the "no changes expected"?

@davidszotten davidszotten force-pushed the issue_278 branch 2 times, most recently from 5cd0bc6 to 01f3766 Compare March 7, 2023 13:49
@davidszotten
Copy link
Author

updated. i did find myself wishing for a command that could output a list of all available fixers but that's a bit tricky due to the current argparse structure so left that for now

@davidszotten

This comment was marked as spam.

Sign in to view
@UnknownPlatypus UnknownPlatypus mentioned this pull request Jul 24, 2023
Merged
ulgens
ulgens reviewed Feb 12, 2024
View reviewed changes
src/django_upgrade/main.py Outdated Show resolved Hide resolved
adamchainz
adamchainz requested changes Feb 12, 2024
View reviewed changes
Copy link
Owner

@adamchainz adamchainz left a comment

Choose a reason for hiding this comment

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

Thank you for starting this @davidszotten. I deferred reviewing because I want this to come in not as a piecemeal feature, but fully formed so it covers most use cases.

I have just rebased the branch and plan to continue work later. I think we should still add the following:

  • Rename the option to --only, since it selects only some fixers (or some other suggestion?)
  • Add an opposing --skip option to skip named fixers. This would users work around when one fixer makes bad changes in their code base.
  • Add a --list-fixers option to list the fixers.
  • Documentation for all the above.

@gav-fyi
Copy link
Contributor

gav-fyi commented Apr 8, 2024

Hi, all.

I was just wondering what the current state of this PR is?

I can't tell if it's currently with David, Adam, or looking for volunteers to implement the additions mentioned above (happy to volunteer if it's the latter).

This feature would have come in useful recently as I upgraded a few projects to 5.0.

@adamchainz
Copy link
Owner

I'm not actively working on it and it doesn't seem like David is either. If you want to make a new PR extending this one with the remaining pieces that I listed, I'd be glad of the help.

gav-fyi added a commit to gav-fyi/django-upgrade that referenced this pull request Apr 9, 2024
@gav-fyi
Implement --skip, --only, and --list-fixers
8c1f038 
* Add ability to run specific fixers with ``--only <fixer_name>``

* Add ability to skip specific fixers with ``--skip <fixer_name>``

* Add ability to list all possible fixers with ``--list-fixers``

  Thanks to Gav O'Connor in `PR #TODO <TODO>`__.

  Thanks to David Szotten in `PR adamchainz#333 <https://github.com/adamchainz/django-upgrade/pull/333>`__.
@gav-fyi gav-fyi mentioned this pull request Apr 9, 2024
Closed
gav-fyi added a commit to gav-fyi/django-upgrade that referenced this pull request Apr 9, 2024
@gav-fyi
Implement --skip, --only, and --list-fixers
25f94fc 
* Add ability to run specific fixers with ``--only <fixer_name>``

* Add ability to skip specific fixers with ``--skip <fixer_name>``

* Add ability to list all possible fixers with ``--list-fixers``

  Thanks to Gav O'Connor in `PR #TODO <TODO>`__.

  Thanks to David Szotten in `PR adamchainz#333 <https://github.com/adamchainz/django-upgrade/pull/333>`__.
gav-fyi added a commit to gav-fyi/django-upgrade that referenced this pull request Apr 9, 2024
@gav-fyi
Implement --skip, --only, and --list-fixers
04f735c 
* Add ability to run specific fixers with ``--only <fixer_name>``

* Add ability to skip specific fixers with ``--skip <fixer_name>``

* Add ability to list all possible fixers with ``--list-fixers``

  Thanks to Gav O'Connor in `PR #TODO <TODO>`__.

  Thanks to David Szotten in `PR adamchainz#333 <https://github.com/adamchainz/django-upgrade/pull/333>`__.
gav-fyi added a commit to gav-fyi/django-upgrade that referenced this pull request Apr 9, 2024
@gav-fyi
Implement --skip, --only, and --list-fixers
7e90415 
* Add ability to run specific fixers with ``--only <fixer_name>``

* Add ability to skip specific fixers with ``--skip <fixer_name>``

* Add ability to list all possible fixers with ``--list-fixers``

  Thanks to Gav O'Connor in `PR #TODO <TODO>`__.

  Thanks to David Szotten in `PR adamchainz#333 <https://github.com/adamchainz/django-upgrade/pull/333>`__.
gav-fyi added a commit to gav-fyi/django-upgrade that referenced this pull request Apr 9, 2024
@gav-fyi
Implement --skip, --only, and --list-fixers
b20edcd 
* Add ability to run specific fixers with ``--only <fixer_name>``

* Add ability to skip specific fixers with ``--skip <fixer_name>``

* Add ability to list all possible fixers with ``--list-fixers``

  Thanks to Gav O'Connor in `PR #TODO <TODO>`__.

  Thanks to David Szotten in `PR adamchainz#333 <https://github.com/adamchainz/django-upgrade/pull/333>`__.
gav-fyi added a commit to gav-fyi/django-upgrade that referenced this pull request Apr 9, 2024
@gav-fyi
Implement --skip, --only, and --list-fixers
8b85c09 
* Add ability to run specific fixers with ``--only <fixer_name>``

* Add ability to skip specific fixers with ``--skip <fixer_name>``

* Add ability to list all possible fixers with ``--list-fixers``

  Thanks to Gav O'Connor in `PR #TODO <TODO>`__.

  Thanks to David Szotten in `PR adamchainz#333 <https://github.com/adamchainz/django-upgrade/pull/333>`__.
gav-fyi added a commit to gav-fyi/django-upgrade that referenced this pull request Apr 9, 2024
@gav-fyi
Implement --skip, --only, and --list-fixers
325bc4d 
* Add ability to run specific fixers with ``--only <fixer_name>``

* Add ability to skip specific fixers with ``--skip <fixer_name>``

* Add ability to list all possible fixers with ``--list-fixers``

  Thanks to Gav O'Connor in `PR #TODO <TODO>`__.

  Thanks to David Szotten in `PR adamchainz#333 <https://github.com/adamchainz/django-upgrade/pull/333>`__.
gav-fyi added a commit to gav-fyi/django-upgrade that referenced this pull request Apr 9, 2024
@gav-fyi
Implement --skip, --only, and --list-fixers
f11b8c2 
* Add ability to run specific fixers with ``--only <fixer_name>``

* Add ability to skip specific fixers with ``--skip <fixer_name>``

* Add ability to list all possible fixers with ``--list-fixers``

  Thanks to Gav O'Connor in `PR #TODO <TODO>`__.

  Thanks to David Szotten in `PR adamchainz#333 <https://github.com/adamchainz/django-upgrade/pull/333>`__.
gav-fyi added a commit to gav-fyi/django-upgrade that referenced this pull request Apr 9, 2024
@gav-fyi
Implement --skip, --only, and --list-fixers
eae6c33 
* Add ability to run specific fixers with ``--only <fixer_name>``

* Add ability to skip specific fixers with ``--skip <fixer_name>``

* Add ability to list all possible fixers with ``--list-fixers``

  Thanks to Gav O'Connor in `PR #TODO <TODO>`__.

  Thanks to David Szotten in `PR adamchainz#333 <https://github.com/adamchainz/django-upgrade/pull/333>`__.
gav-fyi added a commit to gav-fyi/django-upgrade that referenced this pull request Apr 9, 2024
@gav-fyi
Implement --skip, --only, and --list-fixers
3f7f568 
* Add ability to run specific fixers with ``--only <fixer_name>``

* Add ability to skip specific fixers with ``--skip <fixer_name>``

* Add ability to list all possible fixers with ``--list-fixers``

  Thanks to Gav O'Connor in `PR #TODO <TODO>`__.

  Thanks to David Szotten in `PR adamchainz#333 <https://github.com/adamchainz/django-upgrade/pull/333>`__.
gav-fyi added a commit to gav-fyi/django-upgrade that referenced this pull request Apr 9, 2024
@gav-fyi
Implement --skip, --only, and --list-fixers
6b87b62 
* Add ability to run specific fixers with ``--only <fixer_name>``

* Add ability to skip specific fixers with ``--skip <fixer_name>``

* Add ability to list all possible fixers with ``--list-fixers``

  Thanks to Gav O'Connor in `PR #TODO <TODO>`__.

  Thanks to David Szotten in `PR adamchainz#333 <https://github.com/adamchainz/django-upgrade/pull/333>`__.
@gav-fyi gav-fyi mentioned this pull request Apr 9, 2024
Closed
gav-fyi added a commit to gav-fyi/django-upgrade that referenced this pull request Apr 9, 2024
@gav-fyi
Implement --skip, --only, and --list-fixers
9406844 
* Add ability to run specific fixers with ``--only <fixer_name>``

* Add ability to skip specific fixers with ``--skip <fixer_name>``

* Add ability to list all possible fixers with ``--list-fixers``

  Thanks to Gav O'Connor in `PR #TODO <TODO>`__.

  Thanks to David Szotten in `PR adamchainz#333 <https://github.com/adamchainz/django-upgrade/pull/333>`__.
gav-fyi added a commit to gav-fyi/django-upgrade that referenced this pull request Apr 9, 2024
@gav-fyi
Implement --skip, --only, and --list-fixers
18bc20f 
* Add ability to run specific fixers with ``--only <fixer_name>``

* Add ability to skip specific fixers with ``--skip <fixer_name>``

* Add ability to list all possible fixers with ``--list-fixers``

  Thanks to Gav O'Connor in `PR #TODO <TODO>`__.

  Thanks to David Szotten in `PR adamchainz#333 <https://github.com/adamchainz/django-upgrade/pull/333>`__.
@gav-fyi gav-fyi mentioned this pull request Apr 9, 2024
Merged
@gav-fyi
Copy link
Contributor

gav-fyi commented Apr 9, 2024

Sorry for the spam ☝🏻 I forgot I'd referenced this PR early on in my WIP PR description. 😬

adamchainz pushed a commit to gav-fyi/django-upgrade that referenced this pull request May 10, 2024
@gav-fyi @adamchainz
Implement --skip, --only, and --list-fixers
9270b06 
* Add ability to run specific fixers with ``--only <fixer_name>``

* Add ability to skip specific fixers with ``--skip <fixer_name>``

* Add ability to list all possible fixers with ``--list-fixers``

  Thanks to Gav O'Connor in `PR #TODO <TODO>`__.

  Thanks to David Szotten in `PR adamchainz#333 <https://github.com/adamchainz/django-upgrade/pull/333>`__.
@adamchainz
Copy link
Owner

Done in #443.

@adamchainz adamchainz closed this May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ulgens ulgens ulgens left review comments

@adamchainz adamchainz adamchainz requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Option to select specific fixer to run
4 participants
@davidszotten @gav-fyi @adamchainz @ulgens