-
Notifications
You must be signed in to change notification settings - Fork 2
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
Update README.md #6
Conversation
I was setting this up with Synology Container Manager + docker-compose.yml seemed like it wasn't working due to some networking issue. Like a buffoon, I didn't realize that the `labels:` are required and that's why I was seeing `No containers found` in the widget display. I think maybe the project `README.md` could be made more clear by: - stating the label is required and having the [included example](https://github.com/DVDAndroid/glance-docker-container-ext?tab=readme-ov-file#configuration) use the assumed existence of a `Glance` container/project, rather than `Sonar`, since it's a more obvious example - add a commented out line at the bottom of the `docker-compose.yml` example in the [instructions](https://github.com/DVDAndroid/glance-docker-container-ext?tab=readme-ov-file#installation), like `# labels: [...]`, so that users realize there's more required fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contribution; I have some doubts on these changes.
I've tried to write the docs the more generic possible
## Configuration (required) | ||
|
||
Then, for every container you want to monitor, add the following labels to its compose file: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, configuration is required if you want to make widget appear in the homepage; in fact the sentence below declares that "for every container you want to monitor" you need to add some labels.
I'll pass on this
glance.0.name: Glance | ||
glance.0.description: A self-hosted dashboard that puts all your feeds in one place | ||
glance.0.url: https://glance.example.com | ||
glance.0.icon: https://avatars.githubusercontent.com/u/159397742?s=48&v=4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's not the right example to add.
Nobody will have a widget pointing to glance itself; also this may result confusing for users; they may ask theirself: should I put this on glance container?
I put Sonarr as example to say "your container XYZ". A more generic widget should be provided as example.
What a good example could be? I declare all available properties below, so an example like this
glance.X.name: <name of the container>
isn't quite right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, I can see how this might be more confusing. Thanks for explaining your thinking behind the Sonarr example.
I was setting this up with Synology Container Manager + docker-compose.yml seemed like it wasn't working due to some networking issue.
Like a buffoon, I didn't realize that the
labels:
are required and that's why I was seeingNo containers found
in the widget display.I think maybe the project
README.md
could be made more clear by:Glance
container/project, rather thanSonar
, since it's a more obvious exampledocker-compose.yml
example in the instructions, like# labels: [...]
, so that users realize there's more required fieldsglance.X.group
from the included example to simplify things to the minimum needed to work with a single container (this removes the need to updateglance.yml
with thegroup
, which should avoid issues like #4