Skip to content

✨ [#139] Integrate django-upgrade-check#617

Merged
danielmursa-dev merged 1 commit intomasterfrom
feature/139-Integrate-django-upgrade-check
Jun 12, 2025
Merged

✨ [#139] Integrate django-upgrade-check#617
danielmursa-dev merged 1 commit intomasterfrom
feature/139-Integrate-django-upgrade-check

Conversation

@OlhaZahoruiko
Copy link
Contributor

@OlhaZahoruiko OlhaZahoruiko commented Jun 5, 2025

Fixes maykinmedia/open-api-framework#139

Changes

[Describe the changes here]

@codecov-commenter
Copy link

codecov-commenter commented Jun 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.66%. Comparing base (37796bb) to head (4edbe8d).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #617   +/-   ##
=======================================
  Coverage   94.66%   94.66%           
=======================================
  Files         148      148           
  Lines        5171     5171           
=======================================
  Hits         4895     4895           
  Misses        276      276           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@OlhaZahoruiko OlhaZahoruiko marked this pull request as ready for review June 5, 2025 13:47
@OlhaZahoruiko OlhaZahoruiko requested a review from Floris272 June 5, 2025 13:47
@OlhaZahoruiko OlhaZahoruiko force-pushed the feature/139-Integrate-django-upgrade-check branch from f1c1920 to 5655c8a Compare June 6, 2025 13:50
@OlhaZahoruiko OlhaZahoruiko requested review from Floris272 and danielmursa-dev and removed request for Floris272 June 6, 2025 15:16
@OlhaZahoruiko OlhaZahoruiko force-pushed the feature/139-Integrate-django-upgrade-check branch 3 times, most recently from aca3994 to e7fb20e Compare June 10, 2025 15:06
Copy link
Contributor

@Floris272 Floris272 left a comment

Choose a reason for hiding this comment

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

In conf/base.py upgrade_check needs to added to INSTALLED_APPS
and the following needs to be added:

UPGRADE_CHECK_PATHS: UpgradePaths = {
    "<NEXT_VERSION_HERE>": UpgradeCheck(VersionRange(minimum="<CURRENT_VERSION_HERE>")),
}

Sergei also mentioned to add the upgrade simulation: https://github.com/maykinmedia/open-klant/pull/404/files (versions need to be changed)


UPGRADE_CHECK_STRICT = False

Sergei also mentioned to add the upgrade_simulation job https://github.com/maykinmedia/open-klant/pull/404/files

@OlhaZahoruiko
Copy link
Contributor Author

OlhaZahoruiko commented Jun 11, 2025

In conf/base.py upgrade_check needs to added to INSTALLED_APPS and the following needs to be added:

UPGRADE_CHECK_PATHS: UpgradePaths = {
    "<NEXT_VERSION_HERE>": UpgradeCheck(VersionRange(minimum="<CURRENT_VERSION_HERE>")),
}

Sergei also mentioned to add the upgrade simulation: https://github.com/maykinmedia/open-klant/pull/404/files (versions need to be changed)


UPGRADE_CHECK_STRICT = False

Sergei also mentioned to add the upgrade_simulation job https://github.com/maykinmedia/open-klant/pull/404/files

@danielmursa-dev, you mentioned yesterday that I should remove it from INSTALLED_APPS and PATHS.
@Floris272 and @danielmursa-dev would it be possible for you to agree on one approach?

@danielmursa-dev
Copy link
Contributor

@Floris272 the library is already installed in open-api-framework, so all projects can simply extend the INSTALLED_APPS from oaf.
Instead for the settings.py, I think now it is not necessary because at the moment, except open-klant, there is no mandatory versions that the projects are forced to update before the new ones.
So now the library installation is sufficient, to keep track of versions and in the future if it is needed it will be added in the settings

@@ -1,5 +1,3 @@
from pathlib import Path
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this line deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was removed to fix a ruff formatting error: "Import block is un-sorted or un-formatted."

Copy link
Contributor

Choose a reason for hiding this comment

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

Mmmm weird because the Path is used within the file, for example here:

"filename": Path(LOGGING_DIR) / "celery.log",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added it back

Copy link
Contributor

@danielmursa-dev danielmursa-dev left a comment

Choose a reason for hiding this comment

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

OAF 0.10.2 has just been released, so you can run ./bin/compile_dependencies.sh to update the requirements, the same thing for the other projects as well, then everything is fine

@OlhaZahoruiko OlhaZahoruiko force-pushed the feature/139-Integrate-django-upgrade-check branch from 0e9e603 to e7fb20e Compare June 11, 2025 13:20
@OlhaZahoruiko OlhaZahoruiko force-pushed the feature/139-Integrate-django-upgrade-check branch from 8a02f5d to 4edbe8d Compare June 11, 2025 13:46
@danielmursa-dev danielmursa-dev merged commit 198bd3d into master Jun 12, 2025
23 checks passed
@danielmursa-dev danielmursa-dev deleted the feature/139-Integrate-django-upgrade-check branch June 12, 2025 09:10
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.

Add django-upgrade-check to all components

4 participants