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

Add warnings for env_file entries not copied #610

Merged
merged 1 commit into from
Nov 8, 2019

Conversation

ulyssessouza
Copy link
Contributor

As 'env_file' entries are not taken in account,
we warn the user to copy that and fix it's
compose file.

- What I did
Add a warning for each env_file entry on each service

- How I did it
By checking the env_file block for each service entry in docker app init

- How to verify it
Create a docker-compose.yml file with:

version: '3.7'
services:
  front:
    image: "nginx"
    env_file:
     - "myenv.env1"
     - "myenv.env2"

Run:

docker app init --compose-file docker-compose.yml myapp

- Description for the changelog
Add warnings for env_file entries not copied

- A picture of a cute animal (not mandatory but encouraged)
sweater-baby-goat

@ulyssessouza ulyssessouza self-assigned this Sep 10, 2019
@ulyssessouza ulyssessouza force-pushed the add-env_file-warning branch 2 times, most recently from b2c349c to ce98e6f Compare September 10, 2019 17:37
@codecov
Copy link

codecov bot commented Sep 10, 2019

Codecov Report

Merging #610 into master will decrease coverage by 0.03%.
The diff coverage is 70.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #610      +/-   ##
==========================================
- Coverage   70.57%   70.53%   -0.04%     
==========================================
  Files          62       62              
  Lines        3208     3248      +40     
==========================================
+ Hits         2264     2291      +27     
- Misses        639      647       +8     
- Partials      305      310       +5
Impacted Files Coverage Δ
internal/commands/init.go 86.66% <100%> (ø) ⬆️
internal/packager/init.go 63.26% <70.37%> (+1.58%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44932b6...d2c567b. Read the comment docs.

internal/packager/init.go Show resolved Hide resolved
@ulyssessouza ulyssessouza force-pushed the add-env_file-warning branch 2 times, most recently from 978c7e5 to 538f3be Compare September 11, 2019 11:45
@ulyssessouza ulyssessouza force-pushed the add-env_file-warning branch 3 times, most recently from 29a4e9c to 67bd40e Compare September 11, 2019 14:50
@ulyssessouza ulyssessouza force-pushed the add-env_file-warning branch 4 times, most recently from 14cf5aa to 70a508f Compare September 13, 2019 10:00
@ulyssessouza ulyssessouza force-pushed the add-env_file-warning branch 2 times, most recently from 728293d to 7617c51 Compare October 8, 2019 09:46
e2e/commands_test.go Outdated Show resolved Hide resolved
@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "add-env_file-warning" git@github.com:ulyssessouza/app.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354452128
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

e2e/commands_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM with small typo fixed

@ulyssessouza ulyssessouza force-pushed the add-env_file-warning branch 4 times, most recently from db4eaaf to d71c87b Compare November 8, 2019 06:22
As 'env_file' entries are not taken in account,
we warn the user to copy that and fix it's
compose file.

Signed-off-by: Ulysses Souza <ulysses.souza@docker.com>
@chris-crone chris-crone merged commit c7927e1 into docker:master Nov 8, 2019
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.

5 participants