Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds Filebrowser #119

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kinduff
Copy link
Contributor

@kinduff kinduff commented Nov 25, 2023

Adds Filebrowser, a self-hosted file managing interface.

@moan0s moan0s added the New Service Adds a new service to the playbook label Nov 29, 2023
@moan0s
Copy link
Member

moan0s commented Nov 29, 2023

Hi so I had a look and tested it. Worked out of box and the role looks good overall, thanks for adding this! 🎉

Some things I noticed (mixed regarding the role and this playbook if I should split it I can do that):

  • Is there a reason we can't use the docker image to modify the configuration but use the binary?
  • Please add the role to the readme (something like powered by kinduff/ansible-docker-filebrowser
  • There doesn't seem to be a reason to use the environment file. If you don't plan on using it you can remove it here, here [https://github.com/kinduff/ansible-docker-filebrowser/blob/85752efa7456cb5091f648dbda2fe07faa833a19/tasks/install.yml#L21] and the template itself
  • As I see it you use this task to create a database file when it doesn't exist. You could possibly simplify this with ansible.builtin.file
- name: Ensure a file is present
  ansible.builtin.file:
    path: /path/foo
    state: touch

If you have any questions feel free to ask here or on matrix :)

@kinduff
Copy link
Contributor Author

kinduff commented Nov 30, 2023

@moan0s Thanks for the feedback! Will work on some of these changes!

@moan0s moan0s mentioned this pull request Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Service Adds a new service to the playbook
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants