Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Updated dependencies#7589

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
adamchainz merged 1 commit intoencode:masterfromsmithdc1:update_dependencies
Apr 5, 2021
Merged

Conversation

@smithdc1
Copy link
Contributor

I've bumped all the dependency versions with the exception of Markdown. Bumping that version causes a test fail (separate PR incoming shortly).

The main change here is bumping Pytest to version 6.

django-filter>=2.2.0, <2.3
pygments==2.7.1
django-guardian==2.3.0
django-filter>=2.4.0, <2.5
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are some requirements semantically ranged, and others aren't? Seems like it would make sense to do them all.

flake8==3.8.3
flake8==3.8.4
flake8-tidy-imports==4.1.0
pycodestyle==2.6.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Each minor flake8 release only works with onepycodestyle release. Imo you can remove this pin, sinceflake8 depends onpycodestyle - and many other requirements

Suggested change
pycodestyle==2.6.0

pytest-cov>=2.7.1
pytest>=6.1,<6.2
pytest-django>=3.10.0,<3.11
pytest-cov>=2.10.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a max here, like the other semantic ranges, at least<3 ?

@smithdc1
Copy link
ContributorAuthor

I've widened some of the ranges:

  • isort has a commitment from 5.1 onwards not to break anything until 6.0
  • django-filter is likely to move to calver in the next release iirc. So bump allow up to 3.0 for now.
  • Pytest I've allowed up to v.7, the plugins also to their next major release as their changelogs all look trivial in recent releases.

I'm less sure about some of the others, we've seen breaking changes within Markdown 3.3 for example.

coreapi & corescheme are archived, so I've left those as pinned versions.

Copy link
Contributor

@adamchainzadamchainz left a comment

Choose a reason for hiding this comment

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

LGTM, just need to fix the conflict with markdown.

@smithdc1
Copy link
ContributorAuthor

@adamchainz hope you had a good Christmas! 🎅

Could I ask you for a (hopefully) final review of this?

pytest-cov>=2.7.1
pytest>=6.1,<7.0
pytest-cov>=2.10.1,<3.0
pytest-django>=3.10.0,<4.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

pytest-django can be 4.1+

@adamchainz
Copy link
Contributor

Sorry I forgot to come back to this@smithdc1 . Could you rebase to fix the conflict and show the latest tests still pass? Then I'd be happy to merge.

@smithdc1
Copy link
ContributorAuthor

Hi Adam,

A change in pygments version 2.7.3 causes a markdown test to fail.

I suspect this is due to the test containing invalid json. The test was added in#5462, and I can't tell if this was deliberate to get the output with errors. It's this line here that I think is incorrect.

"beta: "this is a string"

The image below shows the current output, the output with pygments>=2.7.3 and the output if I add a" to the json in the test.

image

I think we should adjust the test, but I'd appreciate your thoughts.

@lovelydinosaur
Copy link
Contributor

I think we should adjust the test, but I'd appreciate your thoughts.

Yup, agreed.

@smithdc1smithdc1 mentioned this pull requestMar 31, 2021
semantically ranged dependenciesalpahetical order dependencieswiden allowed version rangesSorted imports with isort 5.8
@adamchainz
Copy link
Contributor

It's green so I'll merge. But what happened to the markdown test change? Did a newer pygments release revert the change in output?

@adamchainzadamchainz merged commitd82519b intoencode:masterApr 5, 2021
@adamchainz
Copy link
Contributor

Also for future it could be convenient to move to a setup like I use on my repos: linting with pre-commit, pre-commit.ci running the tools and auto-updating them weekly, and test dependencies pinned with pip-compile, which I update fortnightly with a script.

@smithdc1smithdc1 deleted the update_dependencies branchApril 5, 2021 09:32
@smithdc1
Copy link
ContributorAuthor

But what happened to the markdown test change?

I fixed it in#7892 👍

Thanks for the merge. 🎉

adamchainz added a commit to adamchainz/django-rest-framework that referenced this pull requestApr 5, 2021
Following [my comment here](encode#7589 (comment)) and [Django's own move to pre-commit](https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/#pre-commit-checks).* Add pre-commit config file to run flake8 and isort.* Add extra "common sense" hooks.* Run pre-commit on GitHub actions using the [official action](https://github.com/pre-commit/action/). This is a good way to get up-and-running but it would be better if we activated [pre-commit.ci](https://pre-commit.ci/), which is faster and will auto-update the hooks for us going forwards.* Remove `runtests.py` code for running linting tools.* Remove `runtests.py --fast` flag, since that would now just run `pytest -q`, which can be done with `runtests.py -q` instead.* Remove tox configuration and requirements files for linting.* Update the contributing guide to mention setting up pre-commit.
@adamchainzadamchainz mentioned this pull requestApr 5, 2021
@adamchainz
Copy link
Contributor

Aha yes, great.

adamchainz added a commit that referenced this pull requestApr 5, 2021
Following [my comment here](#7589 (comment)) and [Django's own move to pre-commit](https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/#pre-commit-checks).* Add pre-commit config file to run flake8 and isort.* Add extra "common sense" hooks.* Run pre-commit on GitHub actions using the [official action](https://github.com/pre-commit/action/). This is a good way to get up-and-running but it would be better if we activated [pre-commit.ci](https://pre-commit.ci/), which is faster and will auto-update the hooks for us going forwards.* Remove `runtests.py` code for running linting tools.* Remove `runtests.py --fast` flag, since that would now just run `pytest -q`, which can be done with `runtests.py -q` instead.* Remove tox configuration and requirements files for linting.* Update the contributing guide to mention setting up pre-commit.
sigvef pushed a commit to sigvef/django-rest-framework that referenced this pull requestDec 3, 2022
sigvef pushed a commit to sigvef/django-rest-framework that referenced this pull requestDec 3, 2022
Following [my comment here](encode#7589 (comment)) and [Django's own move to pre-commit](https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/#pre-commit-checks).* Add pre-commit config file to run flake8 and isort.* Add extra "common sense" hooks.* Run pre-commit on GitHub actions using the [official action](https://github.com/pre-commit/action/). This is a good way to get up-and-running but it would be better if we activated [pre-commit.ci](https://pre-commit.ci/), which is faster and will auto-update the hooks for us going forwards.* Remove `runtests.py` code for running linting tools.* Remove `runtests.py --fast` flag, since that would now just run `pytest -q`, which can be done with `runtests.py -q` instead.* Remove tox configuration and requirements files for linting.* Update the contributing guide to mention setting up pre-commit.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@auvipyauvipyauvipy approved these changes

+1 more reviewer

@adamchainzadamchainzadamchainz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@smithdc1@adamchainz@lovelydinosaur@auvipy

[8]ページ先頭

©2009-2025 Movatter.jp