Closed nehalvpatel closed 2 years ago
Updated the Dockerfile entrypoint to use node
instead of ts-node
.
Hi @nehalvpatel,
First, thank you very much for your contribution!
Second, I think it’d be better to have a configuration file that reads information from the environment instead of modifying index.ts
directly. The point of having configuration files as JavaScript is exactly to give that kind of possibility for people who prefer it, but personally I don’t think it’s a good idea, that’s why Kill the Newsletter! doesn’t endorse it directly.
Third, the preferred deployment method for Kill the Newsletter! is caxa and I don’t want to support Docker. People are free to use it, of course, but I don’t want a Dockerfile as part of Kill the Newsletter! itself because I don’t want to support multiple deployment methods. And if I were to support Docker, I think it’d be nicer to put the Kill the Newsletter! executable in the Docker image, instead of basing on a node
image.
For these reasons I’m closing this pull request without merging. Yet, I really appreciate the effort and I think you did a great job! I hope to receive more contributions from you in the future 😀
Best.
All good!
Can you point to some features you'd like help with?
Oh, that’s very nice of you.
Right now I can’t think of anything. Kill the Newsletter! is pretty feature-complete at this point and it’s mostly maintenance. But I’ll reach back when something comes up!
Best!
Usage
Since there is no official image yet, you will have to build it. Clone my fork, open a Terminal in the directory, and run:
Then, to start the service and make it start on reboot:
Maintainers
Line 3 addresses an issue I was facing when
npm ci
tried to install bent.I modified
index.ts
to accept environment variables. They're safely overridden by a configuration file if one is passed in.Let me know if you'd like any adjustments!