feat: Accept all docker compose filenames on default (#138) #141
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
yarjor
changed the title
There was a problem hiding this comment.
Hi, thanks for your PR.
IMO, this is good to match with docker compose specs.
But it is not good to had a breaking change for this (do not check if file exist anymore).
Maybe we can leave the input empty, if so we apply the defaults files and ignore exists check. If input is provided keep the original behavior
Got you, I changed it back to throw an error if a file was not found, and added a separate flow to search for the default compose filenames if the input was empty. This time I also implementing choosing based on the order of preference specified in the official docs (compose > docker-compose, yaml > yml). BTW, do note that the CI action has permission issues because this pull request is coming from a fork, so the actual action currently isn't tested - I made sure the TS tests pass of course. 😊 |
README.md
Outdated
There was a problem hiding this comment.
The readme file is auto generated from action.yml, you don't have to edit it.
However, can you update compose-file input description in action.yml
| @@ -12,7 +12,7 @@ inputs: | |||
| compose-file: | |||
| description: "Path to compose file(s). It can be a list of files. It can be absolute or relative to the current working directory (cwd)." | |||
There was a problem hiding this comment.
Can you update description to explain the new behavior
| // If no compose files are provided, check for all possible compose files by order of preference specified | ||
| // in official docker compose specs | ||
| composeFiles = ["compose.yaml", "compose.yml", "docker-compose.yaml", "docker-compose.yml"].filter((composeFile: string) => { | ||
| const possiblePaths = [join(cwd, composeFile), composeFile]; | ||
|
|
||
| for (const path of possiblePaths) { | ||
| if (existsSync(path)) { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| throw new Error(`Compose file not found in "${possiblePaths.join('", "')}"`); | ||
| }); | ||
| return false; | ||
| }); |
There was a problem hiding this comment.
- Can you move it in a new dedicated private method like getExistingDefaultComposeFile.
- Return the first existing file instead of checking the whole list
- Throws an error if none found
|
|
||
| - name: "Assert: docker-compose.yaml is used" | ||
| run: | | ||
| docker compose -f ./test/docker-compose.yml ps |
There was a problem hiding this comment.
How is it possible it works as the file /test/docker-compose.yml has been renamed?
neilime
force-pushed
the
all-default-names
branch
2 times, most recently
from
ce6dd82 to
f6fda06
Compare
…cker-compose.ya?ml
… given; change default to empty
neilime
force-pushed
the
all-default-names
branch
from
f6fda06 to
38a654f
Compare
getComposeFilesbehavior to not throw an error if a compose file was not found (and only filter it out), and throw an error only if all give files were not found.docker composecommand (in order to minimize confusion when using default parameters) - try to found files with all 4 possible names.docker compose's behavior that should be noted is that if multiple files exists (e.gcompose.yamlanddocker-compose.yml), the action will use both of them whiledocker composewill choose one file and has an order of preference.Issue: #138