search

PerlDancer / Dancer2

Perl Dancer Next Generation (rewrite of Perl Dancer)
http://perldancer.org/
Other
542 stars 274 forks source link

Copy Dockerfile from the right spot #1623

Closed cromedome closed 3 years ago

cromedome commented 3 years ago

I tested this on a different machine than I developed on prior to release, and had some cruft that masked a bug when adding a Dockerfile to a new application. The dockerfile is located off of the dist dir, not off of the skel dir. This minor fix changes that.

I consider this a blocker for the release. Please review ASAP.

cromedome commented 3 years ago

Looks sane - but does beg the question of whether the Dockerfile actually ought to be under the skel/ directory? (If for no other reason, being in the skel directory would let the user point at their own skel directory containing a customised Dockerfile that does their own particular needful, no?)

I don't disagree. I put the Dockerfile in a separate directory because I didn't want to include it automatically when everything from skel gets copied over. Having it in a separate directory gives us a place to include other configuration for Docker should the implementation ever change, if we wanted to add a docker-compose file, different flavors of the Dockerfile, etc.

We could always take the opposite approach and put it in skel and delete it from the new application directory if --docker wasn't specified, but I don't like this approach. People with a custom skel directory could include their own Dockerfile and ignore the option entirely.

bigpresh commented 3 years ago

I don't disagree. I put the Dockerfile in a separate directory because I didn't want to include it automatically when everything from skel gets copied over.

Ah yes, everything from skel comes over automatically whether you're using Docker or not, doesn't it? In that case, that would not be a good place to put this, and the "it gets copied over but then deleted if we're not going to use it feels a bit... I dunno, hacky.

cromedome commented 3 years ago

Ah yes, everything from skel comes over automatically whether you're using Docker or not, doesn't it? In that case, that would not be a good place to put this, and the "it gets copied over but then deleted if we're not going to use it feels a bit... I dunno, hacky.

That's why I didn't do it that way :)

racke commented 3 years ago

It is really small, maybe we can inline it in the CLI module?