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

Conversation

@dan2k3k4
Copy link
Contributor

commented Aug 16, 2019

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 added 2 commits Aug 13, 2019
@dan2k3k4

This comment has been minimized.

Copy link
Contributor Author

commented Aug 16, 2019

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

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

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

@simesy

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2019

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

@dan2k3k4

This comment has been minimized.

Copy link
Contributor Author

commented Aug 20, 2019

@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 amazeeio:master Aug 28, 2019

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details

@dan2k3k4 dan2k3k4 deleted the dan2k3k4:update-dockerfile-naming branch Aug 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.