This repository has been archived by the owner on Nov 21, 2019. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement empty response #1
Implement empty response #1
Changes from 19 commits
878da02
2e2dca9
9e25f73
db8735b
3916a44
67f92b9
40da7ba
e45973a
30a04bd
72d79e9
111e42d
730364d
d127acd
bc4fcf7
fe3bed3
5c073e3
bccd8e2
61a6610
633a128
f889e96
d9c3ffb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And Make?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make is installed everywhere by default. Technically not required but makes running the software locally easier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Cavaet: My knowledge of Make is pretty much just from Symfony starting to adopt it then stopping.)
Not everywhere; Windows didn't but maybe Windows 10 fixed it with its Linux compatibility? There are different flavours of Make too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't seem like a strict (optional?) dependency, if the user can still run the
docker-compose
commands on their ownThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could mention that in order to use the commands below
gnu make
is required. Would that suffice?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the README to include information about
gnu make
. Please check.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can probably push
poetry config
up too when it is installed? Very minorThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we match https://github.com/libero/browser/blob/master/Dockerfile? So
prod
->test
->dev
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm reading this correctly, all of these images will be created when referencing this file. Is that correct? I don't see the advantage of having everything in one file. Could you explain please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could leave the
test
andprod
image for later when we setup a pipelineThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Was just bit concerned that this looked like it would be a
dev
-only container.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is at the moment (doesn't have the code), but it should be transformed into a
prod
container derivative as soon as theprod
container image is there