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

Update Dockerfile naming convention to <purpose>.dockerfile #1190

Merged
merged 4 commits into from
Aug 28, 2019
Merged

Update Dockerfile naming convention to <purpose>.dockerfile #1190

merged 4 commits into from
Aug 28, 2019

Conversation

dan2k3k4
Copy link
Contributor

To improve syntax highlighting we can use the following naming scheme for Dockerfiles: <purpose>.dockerfile such as:

  • db.dockerfile
  • php.dockerfile
  • nginx.dockerfile

The docker-compose.yml files have been updated, along with the sample Dockerfiles and documentation.

@dan2k3k4
Copy link
Contributor Author

Switched PR from using dan2k3k4:master branch to dan2k3k4:update-dockerfile-naming branch as it's bad-practice to create PRs from default branches.

@AlexSkrypnyk
Copy link
Contributor

This change is going to break upgradability of so many websites. IDE should support configuration of file types by mask, e.g. Dockerfile.*

@simesy
Copy link
Contributor

simesy commented Aug 20, 2019

@alexdesignworks I can't think of what will break in an existing project.
References from your docker-compose.yml point to your own Dockerfiles, not Lagoon Dockerfiles. Otherwise they point directly to images, and not be effected by this change.

@simesy
Copy link
Contributor

simesy commented Aug 20, 2019

Do you just mean if you are merging changes from lagoon Dockerfiles upstream into your project?

@dan2k3k4
Copy link
Contributor Author

@alexdesignworks the changes only concern the /docs folder and the /tests folder - both with updated docker-composer.yml files to point to the updated Dockerfile files.


Here's a little more background.

In multi-stage builds, we tend to require multiple Dockerfile files. Some organisations opt for separating these into their own folders such as:

- /root
-- /docker
--- /cli
---- Dockerfile
--- /db
---- Dockerfile
--- /nginx
---- Dockerfile

This is OK, but can be difficult to find "the right file" you want to edit in an IDE as you have to rely on being able to see the path name.

Another solution is to add the context to the Dockerfile name, such as for the extension:

- /root
-- /docker
--- Dockerfile.cli
--- Dockerfile.db
--- Dockerfile.nginx

But this creates another issue: we no longer have syntax highlighting out-of-the-box.

What if we standardise and use <purpose>.dockerfile as the preferred naming convention? Such as:

- /root
-- /docker
--- cli.dockerfile
--- db.dockerfile
--- nginx.dockerfile

The .dockerfile extension already works for syntax highlighting in VS Code using this glob pattern

Also, using a lowercase .dockerfile would match with general naming convention for other filetype extensions. I think we should avoid capitalising .Dockerfile but keeping Dockerfile for extensionless naming (e.g. in the case of only ever requiring the use of one Dockerfile for your project).

@Schnitzel Schnitzel added this to the v1.0.0 RBAC milestone Aug 28, 2019
@Schnitzel Schnitzel merged commit a143bcb into uselagoon:master Aug 28, 2019
@dan2k3k4 dan2k3k4 deleted the update-dockerfile-naming branch August 29, 2019 08:41
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.

4 participants