Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR re-enables the Requestrr app in DockSTARTer by switching documentation, labels, and compose templates from the deprecated LinuxServer image to the actively maintained thomst08 fork, updating metadata, image names, and configuration paths/ports accordingly. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The aarch64 image reference uses
lthomst08/requestrrwhile the x86_64 and badges usethomst08/requestrr; please confirm and align the image name across architectures to avoid pulling from the wrong repo. - You've changed the config volume from
/configto/root/configand introducedREQUESTRR_PORT=4545; double-check that these align with the forked image's expected config path and port usage, and that they remain consistent with how other apps are configured in DockSTARTer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The aarch64 image reference uses `lthomst08/requestrr` while the x86_64 and badges use `thomst08/requestrr`; please confirm and align the image name across architectures to avoid pulling from the wrong repo.
- You've changed the config volume from `/config` to `/root/config` and introduced `REQUESTRR_PORT=4545`; double-check that these align with the forked image's expected config path and port usage, and that they remain consistent with how other apps are configured in DockSTARTer.
## Individual Comments
### Comment 1
<location> `.apps/requestrr/requestrr.x86_64.yml:3` </location>
<code_context>
services:
requestrr<__instance>:
- image: lscr.io/linuxserver/requestrr:${REQUESTRR<__INSTANCE>__TAG?}
+ image: thomst08/requestrr:${REQUESTRR<__INSTANCE>__TAG?}
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider aligning the image repository between aarch64 and x86_64 definitions.
The aarch64 config uses `lthomst08/requestrr` while this uses `thomst08/requestrr`. If that’s not intentional, one arch may pull a different or non-existent image. Please standardize the namespace across architectures if they’re meant to be the same image.
</issue_to_address>
### Comment 2
<location> `.apps/requestrr/requestrr.yml:9` </location>
<code_context>
- PGID=${PGID?}
- PUID=${PUID?}
- TZ=${TZ?}
+ - REQUESTRR_PORT=4545
restart: ${REQUESTRR<__INSTANCE>__RESTART?}
volumes:
</code_context>
<issue_to_address>
**suggestion:** Using a hardcoded port in the environment may reduce configurability.
Hardcoding `REQUESTRR_PORT=4545` limits flexibility if users want multiple instances or a different port. Consider exposing this as an env var (e.g. `${REQUESTRR<__INSTANCE>__PORT?}`) for consistency with the other settings, unless a fixed port is an intentional requirement.
Suggested implementation:
```
- TZ=${TZ?}
- REQUESTRR_PORT=${REQUESTRR<__INSTANCE>__PORT?}
restart: ${REQUESTRR<__INSTANCE>__RESTART?}
```
1. Ensure that `REQUESTRR<__INSTANCE>__PORT` is defined in the corresponding environment (e.g. `.env` or the parent compose file) for each instance.
2. If there is documentation or example configuration for Requestrr, add `REQUESTRR<__INSTANCE>__PORT` to the documented variables with the default value `4545` to preserve the previous behavior as a suggested default.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| services: | ||
| requestrr<__instance>: | ||
| image: lscr.io/linuxserver/requestrr:${REQUESTRR<__INSTANCE>__TAG?} | ||
| image: thomst08/requestrr:${REQUESTRR<__INSTANCE>__TAG?} |
There was a problem hiding this comment.
issue (bug_risk): Consider aligning the image repository between aarch64 and x86_64 definitions.
The aarch64 config uses lthomst08/requestrr while this uses thomst08/requestrr. If that’s not intentional, one arch may pull a different or non-existent image. Please standardize the namespace across architectures if they’re meant to be the same image.
| - PGID=${PGID?} | ||
| - PUID=${PUID?} | ||
| - TZ=${TZ?} | ||
| - REQUESTRR_PORT=4545 |
There was a problem hiding this comment.
suggestion: Using a hardcoded port in the environment may reduce configurability.
Hardcoding REQUESTRR_PORT=4545 limits flexibility if users want multiple instances or a different port. Consider exposing this as an env var (e.g. ${REQUESTRR<__INSTANCE>__PORT?}) for consistency with the other settings, unless a fixed port is an intentional requirement.
Suggested implementation:
- TZ=${TZ?}
- REQUESTRR_PORT=${REQUESTRR<__INSTANCE>__PORT?}
restart: ${REQUESTRR<__INSTANCE>__RESTART?}
- Ensure that
REQUESTRR<__INSTANCE>__PORTis defined in the corresponding environment (e.g..envor the parent compose file) for each instance. - If there is documentation or example configuration for Requestrr, add
REQUESTRR<__INSTANCE>__PORTto the documented variables with the default value4545to preserve the previous behavior as a suggested default.
Pull request
Purpose
Describe the problem or feature in addition to a link to the issues.
Approach
How does this change address the problem?
Open Questions and Pre-Merge TODOs
Check all boxes as they are completed
Learning
Describe the research stage
Links to blog posts, patterns, libraries or addons used to solve this problem
Requirements
Check all boxes as they are completed
Summary by Sourcery
Re-enable and update the Requestrr app to use the maintained forked image and correct metadata.
Enhancements:
Documentation: