melia icon indicating copy to clipboard operation
melia copied to clipboard

feat: ✨ Add new branch to work with Docker

Open gmargriff opened this issue 1 year ago • 6 comments

Hi, I'm working in this project's Docker support. Hope anyone can better test it's compatibility with current client. Had to make a few changes in server files so that it's compatible with Linux + works with docker initialization. It would be better to keep this in a separate Docker branch for now. Thanks!

gmargriff avatar Apr 16 '24 22:04 gmargriff

Thanks for sharing your work. We wouldn't want to merge this into the main branch, but keeping it in a seperate one might be a nice idea. Though there are a couple changes I'm personally not happy with, such as merging all SQL files or removing the console command code entirely. I feel like we'd want to keep as much of the code the same as possible, without major differences between versions/branches.

exectails avatar Apr 18 '24 22:04 exectails

Thanks for sharing your work.

Thank you for your project, dedication and for taking the time to review this fork.

We wouldn't want to merge this into the main branch, but keeping it in a seperate one might be a nice idea.

Yeah, I think it's quite a change of scope to be merged straight to master.

Though there are a couple changes I'm personally not happy with, such as merging all SQL files or removing the console command code entirely.

I agree, I did the SQL merge because it's easier to setup the Docker entrypoint with one file, but there was no good reason to remove the original files. The console commands shouldn't have been removed either, so I've added a feel checks to skip incompatible commands on incompatible systems whille keeping it working as intented on Windows baremetal OS.

I feel like we'd want to keep as much of the code the same as possible, without major differences between versions/branches.

Also agreed. I'll try to keep up!

gmargriff avatar Apr 19 '24 16:04 gmargriff

I believe if those couple things are improved upon, this is basically fine to be merged into a branch, or even master, if it manages to be general purpose enough.

exectails avatar Jul 07 '24 12:07 exectails

fyi, under windows, i had to add vim to the packges of the dockerfile and do :set ff=unix to the start file because it was copying the file to the docker with non-unix CLRF file ending

y4my4my4m avatar Aug 09 '24 07:08 y4my4my4m

https://github.com/gmargriff/melia-docker/pull/4 I made a PR to @gmargriff 's repo (because im using the docker version for convenience) but thought the docker ver should get merged in the main repo too. If the docker doesnt get merged, maybe you'd still like to add my changes.

y4my4my4m avatar Aug 09 '24 10:08 y4my4my4m

Hey, @exectails and @y4my4my4m , thanks for your work in this PR, I've done just enough changes to make autosync available again on Github, but I'm not directly working on this right now since my PC has been down since may/june, just changed the PR to draft to avoid confusion. Through my phone I've been just keeping the repo synced. I've read through the comments here and it all seems reasonable, as soon as I can get back I'll be working on this. Thanks again!

gmargriff avatar Sep 02 '24 22:09 gmargriff