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

Small improvements #110

Merged
merged 1 commit into from
Feb 8, 2024
Merged

Small improvements #110

merged 1 commit into from
Feb 8, 2024

Conversation

hz61p1
Copy link
Contributor

@hz61p1 hz61p1 commented Jan 30, 2024

No description provided.

@mkuf
Copy link
Owner

mkuf commented Jan 30, 2024

Please elaborate on what you are implementing with this PR and what it improves.

@hz61p1
Copy link
Contributor Author

hz61p1 commented Jan 30, 2024

@mkuf

  1. Adding a linter brings in generally accepted guidelines when writing yaml files
  2. Adding docker image overrides via environment variable, so that you don't have to edit docker compose files if you need a specific version

@mkuf
Copy link
Owner

mkuf commented Jan 30, 2024

Adding a linter seems like a good idea.


If one wants to override the Image for a specific service, the existing docker-compose.override.yaml may be used. Do you have a specific usecase in mind, where this is not working?

@hz61p1
Copy link
Contributor Author

hz61p1 commented Jan 31, 2024

@mkuf It works, but this way allows you to override at a higher level and not to copy-paste, but also to set the desired versions in one file (.env) or at the command line level

@mkuf
Copy link
Owner

mkuf commented Jan 31, 2024

This is introducing additional complexity for the end user by adding functionality which is already present via another configuration file.
I understand where you're coming from, but as the number of users directly benefiting from this change is expected to be very low, I'd rather not merge this as is.

If you're limiting this PR to the linting workflow, I'm happy to merge this.

@hz61p1
Copy link
Contributor Author

hz61p1 commented Feb 1, 2024

@mkuf I've updated the pr

@mkuf mkuf merged commit 13560e7 into mkuf:main Feb 8, 2024
1 check passed
@mkuf
Copy link
Owner

mkuf commented Feb 8, 2024

Merged, thanks!

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.

2 participants