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

add quotation around ports in docker-compose.yml #3090

Merged
merged 3 commits into from
Dec 5, 2017

Conversation

bibaijin
Copy link
Contributor

@bibaijin bibaijin commented Dec 4, 2017

Without quotation around ports in docker-compose.yml, an error will occur when docker-compose up:

Creating network "test_gitea" with the default driver                                                                                            
Creating volume "test_gitea" with local driver                                                                                                   
Creating test_server_1 ...                                                                                                                       
Creating test_server_1 ... error
                                                                                                                                                 
ERROR: for test_server_1  Cannot create container for service server: b'invalid port specification: "601342"'

ERROR: for server  Cannot create container for service server: b'invalid port specification: "601342"'
ERROR: Encountered errors while bringing up the project.

According to yaml's specification, : is a special mark, which denotes map, so quotation is necessary to avoid ambiguity.

@codecov-io
Copy link

codecov-io commented Dec 4, 2017

Codecov Report

Merging #3090 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3090      +/-   ##
==========================================
- Coverage    33.3%   33.26%   -0.04%     
==========================================
  Files         273      273              
  Lines       39954    39954              
==========================================
- Hits        13305    13290      -15     
- Misses      24753    24763      +10     
- Partials     1896     1901       +5
Impacted Files Coverage Δ
modules/process/manager.go 76.81% <0%> (-4.35%) ⬇️
models/repo_indexer.go 47.52% <0%> (-3.47%) ⬇️
models/repo_list.go 65.62% <0%> (-1.57%) ⬇️
models/repo.go 38.15% <0%> (-0.19%) ⬇️

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 03eb47b...ecb97cf. Read the comment docs.

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 4, 2017
@lunny
Copy link
Member

lunny commented Dec 4, 2017

@bibaijin any rule reference link from docker?

@bibaijin
Copy link
Contributor Author

bibaijin commented Dec 4, 2017

@lunny Here is the example from docker-compose, although not described explicitly, all of the example port items is enclosed by quotation. And docker/compose#3109 describe this same problem, the solution is adding quotation.

Actually, it's related to yaml specification, not to docker-compose file specification. Here is yaml's referrence for int, in which colon expression is mentioned.

@lunny lunny added this to the 1.4.0 milestone Dec 5, 2017
@lunny lunny added the type/docs This PR mainly updates/creates documentation label Dec 5, 2017
@lunny
Copy link
Member

lunny commented Dec 5, 2017

LGTM

@tboerger tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 5, 2017
@techknowlogick
Copy link
Member

LGTM

@tboerger tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 5, 2017
@lunny lunny merged commit e2968b4 into go-gitea:master Dec 5, 2017
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/docs This PR mainly updates/creates documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants