Migration linting cannot be controlled exclusively through configuration files

[phillipuniverse]

The usage docs says that all of the parameters can be controlled with a configuration file (pyproject.toml or .django_migration_linter.cfg) but this is not quite true. There are a handful of configs that can only be used via arguments on the command line:

• project_root_path
• verbosity
• app_label
• migration_name
• git_commit_id
• include_migrations_from

All of these properties make explicit references to options passed to the management command rather than to config which is read from config files. Example:

django-migration-linter/django_migration_linter/management/commands/lintmigrations.py

Lines 208 to 213 in 8cf37cd

	linter.lint_all_migrations(
	app_label=options["app_label"],
	migration_name=options["migration_name"],
	git_commit_id=options["git_commit_id"],
	migrations_file_path=options["include_migrations_from"],
	)

[vanschelven]

See #189

[David-Wobrock]

Hi @phillipuniverse , hi @vanschelven !

Thanks for the issue and the pull request 👍

I think I didn't add all parameters because I couldn't think of a use cases where one would need to set app_label and the migration_name in the config files, nor the git_commit_id 🤔 Since those are essentially parameters that are related to a specific call to the linter, and not configuration per se

I totally agree with include_migrations_from, project_root_path and verbosity, they make sense to be constant in a config file.

Could you explain to me the use case(s) you have for these params in config files? :) So I get a better grasp of the need

[vanschelven]

Irregardless of any particular use case I'd argue that having support for these params in the config files is simply a matter of good design. First, because it's what the friendly manual states. Second, because it would be the consistent thing to do. This is simply about not violating the principle of least surprise: if I already have 3 options configured in e.g. tox.ini, I'd be tempted to configure a fourth one nearby. Such behaviors not being picked up by the tool then leads to a lot of head scratching during debugging.

But since you asked, my particular use-case for the git-commit-id was since we had a particular version of our application already in production, and wanted to tell this to our CI tool, so that it wouldn't complain about problems that are already in the past. The most generic point of configuration would be tox.ini, since that applies equally to local dev environments across the team, and the CI tool.

[phillipuniverse]

Irregardless of any particular use case I'd argue that having support for these params in the config files is simply a matter of good design. First, because it's what the friendly manual states. Second, because it would be the consistent thing to do. This is simply about not violating the principle of least surprise: if I already have 3 options configured in e.g. tox.ini, I'd be tempted to configure a fourth one nearby. Such behaviors not being picked up by the tool then leads to a lot of head scratching during debugging.

I echo this. Speaking for myself, I was surprised to see there was a difference in how the config parameters were treated. I appreciate the care given to try to give explicit intention to the configs but... it's more work (both through code and through having correct documentation, as is evident by the situation today) and I would say does not add much value. I confess I haven't looked as much at things like migration_name, maybe that one doesn't make any sense to treat statically in a pyproject.toml 🤷 .

But since you asked, my particular use-case for the git-commit-id was since we had a particular version of our application already in production, and wanted to tell this to our CI tool, so that it wouldn't complain about problems that are already in the past

This is also my use case. The goal I was going for was to allow my team to run nothing but manage.py lintmigrations and everything work automatically. Just like @vanschelven, I introduced this into a pre-existing application that had been in production for a long time and will always ignore migrations that were added before this fantastic linter was included.

I totally agree with include_migrations_from, project_root_path and verbosity, they make sense to be constant in a config file.

One other point here about project_root_path. In my use case we have some people that use Docker as a development environment and some that don't. This means the project root path could be /Users/phillip/projects/my_project (for me running on macOS) or /usr/src/my_project when inside of Docker. It would be nice if we could compute this config value through something like settings.py.

Perhaps there are other use cases for config to involve computation. Maybe there could be another option for reading configs and they could be considered in the following order:

1. Read from a MIGRATION_LINTER section in settings.py
2. Read from pyproject.toml/tox.ini/etc
3. Read from command line

So command line arguments always take precedence over anything else.

Hope that helps!

[David-Wobrock]

Okay, thanks you two for the complimentary info 👍
While I don't necessarily agree on all points made, I think it's easiest and most flexible to simply allow all options to be specified in the config files too.
I opened a new PR with the necessary changes #191

@phillipuniverse Interesting remark about the project_root_path as a Python settings. For now, the linter only supports one settings (MIGRATION_LINTER_OVERRIDE_MAKEMIGRATIONS), but one could imagine handling also all options as Python settings if that makes sense.
In a simple pattern like f"MIGRATION_LINTER_{option_key.upper()}" or f"MIGRATION_LINTER_{option_key.upper()}_OPTION".
Can I let you open another issue for this specific discussion? :)

[phillipuniverse]

@David-Wobrock I submitted a new issue at #198