Skip to content

fix: clarify 'warnings as error' behaviour by splitting existing option into two different options #321

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

Open
David-Wobrock wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: clarify 'warnings as error' behaviour by splitting existing option into two different options #321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
## 7.0.0

Feature:
- **Breaking change**: clarify "warning as error" behaviour.
- Option `--warnings-as-errors` (both words are plural) is deleted.
- Instead, use `--all-warnings-as-errors` to treat all warnings as errors.
- Or use `--warning-as-error [MIGRATION_TEST_CODE [...]]` (both words are singular) to specify which linter rules to treat as errors instead of warnings.

## 6.0.0

Feature:
Expand Down
3 changes: 2 additions & 1 deletion docs/makemigrations.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,6 @@ Deleted tests/test_project/app_correct/migrations/0003_a_column.py
Among the options that can be given, additionally to the default `makemigrations` options, you can count the options
to configure what should be linted:
* `--database DATABASE` - specify the database for which to generate the SQL. Defaults to *default*.
* `--warnings-as-errors [MIGRATION_TEST_CASE ...]` - handle warnings as errors. Optionally specify the selected tests using their code.
* `--all-warnings-as-errors` - treat all warnings as errors.
* `--warning-as-error [MIGRATION_TEST_CASE ...]` - treat specific tests as errors. Specify the tests through their code.
* `--exclude-migration-tests MIGRATION_TEST_CODE [...]` - specify backward incompatible migration tests to be ignored using the code (e.g. ALTER_COLUMN).
47 changes: 24 additions & 23 deletions docs/usage.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,29 +26,30 @@ Below the detailed command line options, which can all also be defined using a c

If you are using a config file, replace any dashes (`-`) with an underscore (`_`).

| Parameter | Description |
|-------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `--git-commit-id GIT_COMMIT_ID` | If specified, only migrations since this commit will be taken into account. |
| `--ignore-name-contains IGNORE_NAME_CONTAINS` | Ignore migrations containing this name. |
| `--ignore-name IGNORE_NAME [IGNORE_NAME ...]` | Ignore migrations with exactly one of these names. |
| `--include-name-contains INCLUDE_NAME_CONTAINS` | Include migrations containing this name. |
| `--include-name INCLUDE_NAME [INCLUDE_NAME ...]` | Include migrations with exactly one of these names. |
| `--include-apps INCLUDE_APPS [INCLUDE_APPS ...]` | Check only migrations that are in the specified django apps. |
| `--exclude-apps EXCLUDE_APPS [EXCLUDE_APPS ...]` | Ignore migrations that are in the specified django apps. |
| `--exclude-migration-tests MIGRATION_TEST_CODE [...]` | Specify backward incompatible migration tests to be ignored using the code (e.g. ALTER_COLUMN). |
| `--verbosity or -v {0,1,2,3}` | Print more information during execution. |
| `--database DATABASE` | Specify the database for which to generate the SQL. Defaults to *default*. |
| `--cache-path PATH` | specify a directory that should be used to store cache-files in. |
| `--no-cache` | Don't use a cache. |
| `--applied-migrations` | Only lint migrations that are applied to the selected database. Other migrations are ignored. |
| `--unapplied-migrations` | Only lint migrations that are not yet applied to the selected database. Other migrations are ignored. |
| `--project-root-path DJANGO_PROJECT_FOLDER` | An absolute or relative path to the django project. |
| `--include-migrations-from FILE_PATH` | If specified, only migrations listed in the given file will be considered. |
| `--quiet or -q {ok,ignore,warning,error}` | Suppress certain output messages, instead of writing them to stdout. |
| `--warnings-as-errors [MIGRATION_TEST_CODE [...]]` | Handle warnings as errors and therefore return an error status code if we should. Optionally specify migration test codes to handle as errors. When no test code specified, all warnings are handled as errors. |
| `--sql-analyser` | Specify the SQL analyser that should be used. Allowed values: 'sqlite', 'mysql', 'postgresql'. |
| `--ignore-sqlmigrate-errors` | Ignore failures of sqlmigrate commands. |
| `--ignore-initial-migrations` | Ignore initial migrations. |
| Parameter | Description |
|-------------------------------------------------------|-------------------------------------------------------------------------------------------------------|
| `--git-commit-id GIT_COMMIT_ID` | If specified, only migrations since this commit will be taken into account. |
| `--ignore-name-contains IGNORE_NAME_CONTAINS` | Ignore migrations containing this name. |
| `--ignore-name IGNORE_NAME [IGNORE_NAME ...]` | Ignore migrations with exactly one of these names. |
| `--include-name-contains INCLUDE_NAME_CONTAINS` | Include migrations containing this name. |
| `--include-name INCLUDE_NAME [INCLUDE_NAME ...]` | Include migrations with exactly one of these names. |
| `--include-apps INCLUDE_APPS [INCLUDE_APPS ...]` | Check only migrations that are in the specified django apps. |
| `--exclude-apps EXCLUDE_APPS [EXCLUDE_APPS ...]` | Ignore migrations that are in the specified django apps. |
| `--exclude-migration-tests MIGRATION_TEST_CODE [...]` | Specify backward incompatible migration tests to be ignored using the code (e.g. ALTER_COLUMN). |
| `--verbosity or -v {0,1,2,3}` | Print more information during execution. |
| `--database DATABASE` | Specify the database for which to generate the SQL. Defaults to *default*. |
| `--cache-path PATH` | specify a directory that should be used to store cache-files in. |
| `--no-cache` | Don't use a cache. |
| `--applied-migrations` | Only lint migrations that are applied to the selected database. Other migrations are ignored. |
| `--unapplied-migrations` | Only lint migrations that are not yet applied to the selected database. Other migrations are ignored. |
| `--project-root-path DJANGO_PROJECT_FOLDER` | An absolute or relative path to the django project. |
| `--include-migrations-from FILE_PATH` | If specified, only migrations listed in the given file will be considered. |
| `--quiet or -q {ok,ignore,warning,error}` | Suppress certain output messages, instead of writing them to stdout. |
| `--all-warnings-as-errors` | Treat all warnings as errors. |
| `--warning-as-error [MIGRATION_TEST_CODE [...]]` | Treat specified tests as errors. Specify migration test codes to handle as errors. |
Comment on lines +48 to +49
Copy link
Collaborator Author

@David-Wobrock David-Wobrock Jan 4, 2026

Choose a reason for hiding this comment

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

These rows changed.

| `--sql-analyser` | Specify the SQL analyser that should be used. Allowed values: 'sqlite', 'mysql', 'postgresql'. |
| `--ignore-sqlmigrate-errors` | Ignore failures of sqlmigrate commands. |
| `--ignore-initial-migrations` | Ignore initial migrations. |

## Django settings configuration

Expand Down
15 changes: 3 additions & 12 deletions src/django_migration_linter/management/commands/lintmigrations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@

from ...constants import __version__
from ...migration_linter import MessageType, MigrationLinter
from ..utils import (
configure_logging,
extract_warnings_as_errors_option,
register_linting_configuration_options,
)
from ..utils import configure_logging, register_linting_configuration_options

CONFIG_NAME = "django_migration_linter"
PYPROJECT_TOML = "pyproject.toml"
Expand Down Expand Up @@ -146,11 +142,6 @@ def handle(self, *args, **options):
if not options[k]:
options[k] = v

(
warnings_as_errors_tests,
all_warnings_as_errors,
) = extract_warnings_as_errors_option(options["warnings_as_errors"])

configure_logging(options["verbosity"])

root_path = options["project_root_path"] or os.path.dirname(
Expand All @@ -171,8 +162,8 @@ def handle(self, *args, **options):
only_unapplied_migrations=options["unapplied_migrations"],
exclude_migration_tests=options["exclude_migration_tests"],
quiet=options["quiet"],
warnings_as_errors_tests=warnings_as_errors_tests,
all_warnings_as_errors=all_warnings_as_errors,
warnings_as_errors_tests=options["warning_as_error"],
all_warnings_as_errors=options["all_warnings_as_errors"],
no_output=options["verbosity"] == 0,
analyser_string=options["sql_analyser"],
ignore_sqlmigrate_errors=options["ignore_sqlmigrate_errors"],
Expand Down
18 changes: 5 additions & 13 deletions src/django_migration_linter/management/commands/makemigrations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@

from django_migration_linter import MigrationLinter

from ..utils import (
configure_logging,
extract_warnings_as_errors_option,
register_linting_configuration_options,
)
from ..utils import configure_logging, register_linting_configuration_options


def ask_should_keep_migration() -> bool:
Expand Down Expand Up @@ -50,7 +46,8 @@ def handle(self, *app_labels, **options):
self.lint = options["lint"]
self.database = options["database"]
self.exclude_migrations_tests = options["exclude_migration_tests"]
self.warnings_as_errors = options["warnings_as_errors"]
self.warning_as_error = options["warning_as_error"]
self.all_warnings_as_errors = options["all_warnings_as_errors"]
self.sql_analyser = options["sql_analyser"]
self.ignore_sqlmigrate_errors = options["ignore_sqlmigrate_errors"]
configure_logging(options["verbosity"])
Expand Down Expand Up @@ -79,19 +76,14 @@ def write_migration_files(self, changes: dict[str, list[Migration]]) -> None:
else default_should_keep_migration
)

(
warnings_as_errors_tests,
all_warnings_as_errors,
) = extract_warnings_as_errors_option(self.warnings_as_errors)

# Lint migrations.
linter = MigrationLinter(
path=os.environ["DJANGO_SETTINGS_MODULE"],
database=self.database,
no_cache=True,
exclude_migration_tests=self.exclude_migrations_tests,
warnings_as_errors_tests=warnings_as_errors_tests,
all_warnings_as_errors=all_warnings_as_errors,
warnings_as_errors_tests=self.warning_as_error,
all_warnings_as_errors=self.all_warnings_as_errors,
analyser_string=self.sql_analyser,
ignore_sqlmigrate_errors=self.ignore_sqlmigrate_errors,
)
Expand Down
28 changes: 10 additions & 18 deletions src/django_migration_linter/management/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import argparse
import logging

from django.core.management import CommandParser
Expand All @@ -26,11 +27,17 @@ def register_linting_configuration_options(parser: CommandParser) -> None:
)

parser.add_argument(
"--warnings-as-errors",
"--all-warnings-as-errors",
action=argparse.BooleanOptionalAction,
default=False,
help="treat all warnings as errors",
)
parser.add_argument(
"--warning-as-error",
type=str,
nargs="*",
help="handle warnings as errors. Optionally specify the tests to handle as "
"errors (e.g. RUNPYTHON_REVERSIBLE)",
help="treat specified tests as errors. Specify the tests through their code "
"(e.g. RUNPYTHON_REVERSIBLE)",
)

parser.add_argument(
Expand Down Expand Up @@ -62,18 +69,3 @@ def configure_logging(verbosity: int) -> None:
logger.disabled = True
else:
logging.basicConfig(format="%(message)s")


def extract_warnings_as_errors_option(
warnings_as_errors: list[str] | None,
) -> tuple[list[str] | None, bool]:
if isinstance(warnings_as_errors, list):
warnings_as_errors_tests = warnings_as_errors
# If the option is specified but without any test codes,
# all warnings become errors
all_warnings_as_errors = len(warnings_as_errors) == 0
else:
warnings_as_errors_tests = None
all_warnings_as_errors = False

return warnings_as_errors_tests, all_warnings_as_errors