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.

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

Rename '--config' to '--extra-settings' for clarity and avoid conflicts #76

Merged
merged 4 commits into from
Sep 8, 2022

Conversation

PeterJCLaw
Copy link
Contributor

@PeterJCLaw PeterJCLaw commented Sep 7, 2022

This works around django-configurations interception of arguments which are contractions of its --configuration argument (jazzband/django-configurations#343).

We also take the opportunity to slightly clarify the name of this argument.

Have tested locally that the impacted commands still behave as expected using the new spelling of the argument.

This works around 'django-configurations' interception of arguments
which are contractions of its '--configuration' argument. We also
take the opportunity to slightly clarify the name of this argument.

See jazzband/django-configurations#343
@PeterJCLaw PeterJCLaw marked this pull request as ready for review September 7, 2022 16:13
Also clarify how these settings are applied.
@PeterJCLaw PeterJCLaw requested a review from lirsacc September 7, 2022 17:43
Copy link
Contributor

@lirsacc lirsacc left a comment

Choose a reason for hiding this comment

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

One thought: from the behaviour should this be named something closer to overrides to make it clear it's not just additive? (Don't feel too strongly about it though)

README.md Outdated Show resolved Hide resolved
Co-authored-by: Charles Lirsac <[email protected]>
@PeterJCLaw
Copy link
Contributor Author

One thought: from the behaviour should this be named something closer to overrides to make it clear it's not just additive?

Yeah, that might be better. I expect it would need to be --override-settings or similar rather than the more natural --settings-overrides to avoid clashes with Django's built-in --settings flag that django-configurations behaviour would similarly mangle.

Will revisit this before merging tomorrow.

@PeterJCLaw
Copy link
Contributor Author

I've improved the inline documentation of the CLI arguments, but I'm going to stick with the name for now.

@PeterJCLaw PeterJCLaw merged commit a401c57 into master Sep 8, 2022
@PeterJCLaw PeterJCLaw deleted the rename-extra-config branch September 8, 2022 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants