search

RMI-PACTA / docker

Docker images
https://2degreesinvesting.github.io/docker/
MIT License
1 stars 0 forks source link

Expand '.' to the more informative top level name #34

Closed maurolepore closed 3 years ago

maurolepore commented 3 years ago

Fixes #15

This PR produces a more informative message when the given tag already exists in the repo.

Thanks @cjyetman

maurolepore commented 3 years ago

Thanks! I'm not partial to using repo/ but in adding the "/" I was inspired by the -F flag to ls:

➜  docker git:(30_remove-user_results) ls   
docker.Rproj  pacta  README.md  r-packages  system
➜  docker git:(30_remove-user_results) ls -F
docker.Rproj  pacta/  README.md  r-packages/  system/

Can you explain a bit why you think its not appropriate? I see how it may be inconsistent with other references to a directory, and maybe with how git itself refers to repos. I'm just curuious.

cjyetman commented 3 years ago
  1. it's referred to as a "repository" which I take to mean that it is the name of a repository not the name of a directory, and as far as I know a repository name does not by default include a trailing "/"
  2. imo trailing slashes do not belong at the end of a directory name either... the "/" is syntax, not part of the name... e.g. look at the result of fs::path("foo", "bar", "baz").... and also, consider the struggles we've had with paths being copy-pasted together and it being uncertain whether or not it had a trailing "/".... also consider that the character used for slash is not the same on all platforms
cjyetman commented 3 years ago

wrt to ls -F... -F is a special flag used to classify the type of file it's showing...

  1. that is not the default behavior, and I would take the default behavior as probably the most correct (not printing trailing slash)
  2. it's a convenience that helps the user read a list of files and/or directories and classify them quickly, but I think loses its purpose in the context of a single name being printed
maurolepore commented 3 years ago

Thanks a lot!