Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Corrected SYNTAX some fixes #1468

Closed
wants to merge 1 commit into from
Closed

Corrected SYNTAX some fixes #1468

wants to merge 1 commit into from

Conversation

frank-dspeed
Copy link
Contributor

Commit Message Guidelines

Header
Blank Line
Body
Blank Line
Footer

The header should look like:
<type>(<scope>): <subject>

The body should have any necessary detailed info about the commit:
An example, references as to where this idea came from, etc.

The footer should have all the issues tagged:
Fixes #123, Fixes #456

So a commit should look like:
feat(users): Add new Yahoo authentication

Yahoo authentication idea proposed by @codydaig
Example implementation in file.js

Fixes #82
  • Types:
    • feat - Features, Enhancements, and overall Improvements
    • fix - Fixes, Bugs, HotFixs, etc...
    • doc - Changes to the Documentation and doesn't actually touch any code.
  • Scope:
    • The scope should be where the change took place.
    • Examples: users, core, config, articles
  • Subject:
    • The subject line should be clear and concise as to what is being accomplished in the commit.
  • General Rules:
    • No Line in the Commit message can be longer than 80 characters.
  • Reference: Angular Conventions

You did a lot of overhead now the resulting images are much smaller and it will build much faster
greetings.

You did a lot of overhead now the resulting images are much smaller and it will build much faster
greetings.
@lirantal
Copy link
Member

@frank-dspeed care to explain?

@lirantal lirantal self-assigned this Aug 30, 2016
@lirantal lirantal added this to the 0.5.0 milestone Aug 30, 2016
@hyperreality
Copy link
Contributor

Looks like he improved the Docker build process, but the message is certainly misleading and a mess. I'm not running Docker but could somebody who is test this?

@frank-dspeed
Copy link
Contributor Author

i can explain a bit every RUN Instruction runs in its own Layer and Container on docker build so every added run instruction adds a layer example i do RUN apt-get update and then RUN apt-get install and after this RUN apt-get clean then !!!! now the magic happens apt-get clean is useless because the apt-get list is still in the 2 layers before but when i for example do RUN apt-get update && apt-get install git && apt-get clean then its 1 layer that will not include the apt mirror and packet lists because i removed them befor the build container got commited!

@frank-dspeed
Copy link
Contributor Author

Then the next change docker syntax don't accepts EXPOSE 80:80 and this stuff because its simply not allowed to map host ports for security reason sure you can do this on docker run but not in docker build

also every single line EXPOSE ends up in its own build container and image layer so the logic solution is EXPOSE 80 443 and so on
EXPOSE is only for linking some info to the docker daemon and user that on this ports something could run

but sorry that i got not much more time to explain maybe read the Docker Docs about building images to get deeper into that

or simply do docker export yourimage > bla.tar
then build my version do docker export myimage > bla2.tar
and see the diff between that results also mesure build time

@lirantal
Copy link
Member

@frank-dspeed got it. Though I don't see any issue with the separate RUN layers, if it overall simplifies the build process then it's ok with me. Did you test this changed Dockerfile with the rest of the docker setups that were recently added?

@FedeG how does this play well with your recent Dockerfile-production and updates to the docker-compose.yml file?

@FedeG
Copy link
Contributor

FedeG commented Aug 31, 2016

This Dockerfile is very simplified but some layers were important for developers.
For example:
1- Layer npm before ADD . /opt/mean.js.
Code:

ADD package.json /opt/mean.js/package.json
RUN npm install --quiet

If a developer changes files in proyect but don't modify package.json, this layers are caching in future builds.

2- Layer bower before ADD . /opt/mean.js.
Code:

ADD bower.json /opt/mean.js/bower.json
ADD .bowerrc /opt/mean.js/.bowerrc
RUN bower install --quiet --allow-root --config.interactive=false

If a developer changes files in proyect but don't modify bower.json or .bowerrc, this layers are caching in future builds.

If docker have npm/bower commands in cache, the future builds are very fast.

@FedeG
Copy link
Contributor

FedeG commented Aug 31, 2016

For developers is very important measure build time for successive builds.
In your dockerfile, the time is equal in successive builds but in the old dockerfile the time is minor because the old dockerfile used caching.

@FedeG
Copy link
Contributor

FedeG commented Aug 31, 2016

@lirantal
Copy link
Member

lirantal commented Sep 1, 2016

@FedeG @frank-dspeed can you guys please work to fix this PR with both of your comments?

FedeG pushed a commit to FedeG/mean that referenced this pull request Sep 7, 2016
FedeG pushed a commit to FedeG/mean that referenced this pull request Sep 7, 2016
lirantal pushed a commit that referenced this pull request Sep 9, 2016
* Fix(#1468) - Corrected SYNTAX some fixes

* Fix(#1468) - Fix(#1453) - Corrected SYNTAX some fixes for production and fix this bug #1453

* Fix(#1453) - Add .dockerignore

* Update Dockerfile-production

* Update Dockerfile development
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants