Questions on submitting my patches from Pretendo Docker

Technical Discussion
1

About

In my work on Pretendo Docker, I’ve created a bunch of patches. These patches don’t just Dockerize the servers, many also make some larger changes. I believe that I am ready to start merging them into the upstream servers, and I’d like to discuss what the right strategy for this is. Pinging @PN_Jon and any other developer who wants to add anything.

General strategy

Many of the patches aren’t ready to be submitted as they are, as they need to be changed from hard-coding things that work well with my setup to being configurable. I plan on focusing on one repo at a time, checking over every patch and modifying it if necessary, and then applying each patch as a commit to my fork. Then, when everything’s ready, I’ll submit a PR.

Questionable patches

I have a few patches that I find useful for my server but am unsure whether they would be accepted. I’m going to ask questions about them here since they relate to more than one repo and it’s easier to ask here than open a bunch of GitHub issues.

Environment variables

I’d like to migrate the servers that are still using a config.json (website, Juxtaposition) to environment variables. I don’t think this is too controversial, but I’d like to confirm that it’s okay first.

Debug Dockerfiles

I personally like to have the devcontainer workflow as an option. For all of the servers, I updated the Dockerfile to set up remote debugging inside of the container. Here’s a Node.js example and here’s a Go example.

Obviously, running the servers with debugging enabled all of the time would be unsuitable for production use. Because of this, I would propose creating a second Dockerfile, call it Dockerfile.debug, with debugging options enabled. I hesitate to do this though because I don’t think I’ve ever seen it in other projects. Is this common? Or is this considered bad practice? If so, what better options are there? Clearly, for Node.js, the other option is to override the cmd to add --inspect, but this isn’t an option for Go because it requires installing an extra binary (Delve) for debugging.

Server identification

I created a couple of patches that make it easier to identify when one is connecting to a self-hosted server: one for custom license agreement text and one for a red banner on the website. Would it be acceptable to add both of these as additional disabled-by-default environment variables? For example, PN_ACT_CONFIG_OVERRIDE_LICENSE_TEXT and PN_WEBSITE_CONFIG_WARNING_BANNER.

Cemu fake online files

I understand that this feature was removed in production due to abuse, but that seems like less of a problem for a private server or development. I added the code for the Cemu online files back in a patch, and this also required bypassing console verification for the invalid certificates generated by the fake online files. Is it acceptable to add this feature back if it is disabled by default and clearly marked as dangerous for public use? Or is this a just feature that needs to be dropped?

2 Likes
2

Essentially all of this would fall under either feature request or enhancement issues on the relevant repositories. I would suggest either waiting for the contribution guide PR to merge so templates are active, or manually use the templates like I do, and just submit issues for any changes you’d want. That’s what the templates are for

3

Alright. I had thought to post this here because the changes affect multiple repositories, but if this is the wrong place to discuss, I will just repost the questionable changes as GitHub issues.

4

This was definitely the right place to ask this kind of question :+1: the answer to your question is just to use GitHub issues

1 Like