Skip to content

Conversation

@zQueal
Copy link
Contributor

@zQueal zQueal commented Mar 14, 2025

Updated docker-compose examples for what works out of the box.

Copy link
Contributor

@stappersg stappersg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The additions are good, the changes not so good.

- -d /tmp/rgit-cache.db
volumes:
- /path/to/my-repos:/git
- /volume/git:/git
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's going to be different for each environment, so the difference between the two paths is completely inconsequential. /volume doesn't exist in a standard environment, and either does /path/to/my-repos, so there's no danger of exposing anything here.

I don't understand your gripe.

- /volume/git:/git
ports:
- 3333:8000
- 8000:8000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ports shouldn't be exposed at all in a docker environment--but this is how people use docker, and is the way that the original docker-composer.yml file was to begin with. Yet again, it's going to depend on the end-users environment, so using 3333:8000 is tentatively identical to 8000:8000.

3333 interferes with NCID servers on the host machine, and 8000 interferes with some database servers. There's effectively nothing you can set it to that won't interfere with something, so again, I really don't understand the gripe here. They're both non-privileged ports.

@stappersg
Copy link
Contributor

stappersg commented Mar 15, 2025 via email

@zQueal
Copy link
Contributor Author

zQueal commented Mar 16, 2025

So then don't merge it? I still, again and again, fail to see the issue. Fact of the matter is, is that the docker example that's currently in this repository doesn't work. So it kinda is what it is. Update it or don't. 🤷🏻‍♂️

@zQueal zQueal closed this Mar 16, 2025
@stappersg
Copy link
Contributor

stappersg commented Mar 16, 2025 via email

@w4
Copy link
Owner

w4 commented Mar 16, 2025

Hi @zQueal, thanks for your contribution. Looks perfectly fine to me. Ignore the argumentative other commenter, just a random member of the public that added a favicon 😄

@w4 w4 reopened this Mar 16, 2025
@w4 w4 merged commit ea0996b into w4:main Mar 16, 2025
1 check passed
@zQueal
Copy link
Contributor Author

zQueal commented Mar 16, 2025

Appreciate the work man, love this project!

@stappersg
Copy link
Contributor

stappersg commented Apr 6, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants