-
-
Notifications
You must be signed in to change notification settings - Fork 106
Create docker-transmission-openvpn application #866
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
base: main
Are you sure you want to change the base?
Create docker-transmission-openvpn application #866
Conversation
@SunFlowerOwl Why do you request a review from me? |
As I was preparing my upcoming push, I was looking for a feature that would let me list remarks and show the corresponding code that resolves them, instead of just marking them as resolved after the push. Since this is my first ever pull request on GitHub, I thought that button was for that purpose — but I later realized it wasn’t, and it sent you a re-review request without confirmation and with no way to cancel. Sorry about that. The push is coming soon; I just need to fix some issues with the cron job after testing on my Proxmox installation. |
alright, sure no worries ^^ |
@SunFlowerOwl please mark the review commands that you believe you solved as resolved. |
Alright, I was unsure if it was on my side.
^^ sometimes curiosity |
✅ Test Checklist
|
✅ Test Checklist
|
I'm sorry, i just realised, while doing some checks on the repositories you pull from. These are apps without a commit in 6 or more years. I can't allow such apps to be included in our repo, if nothing else but for security reasons. @community-scripts/contributor |
Hi, Thank you for your multiple review with @CrazyWolf13 , I think you’re talking about the Transmission UI. No worries... I was also surprised. That may explain why they don’t have a release repo on their GitHub. For me, these WebUIs are not mandatory, and the original Transmission UI is more than enough to make it work. I can remove them if you think it could be acceptable without. |
@tremor021 I’ve removed them all, and I think it’s better this way since the WebUIs are only cosmetic. The main app itself hasn’t received any commits for three months. Is that acceptable for you? Just to let you know: The UIs are still compatible, but users will need to add them on their own and at their own risk. ✅ Test Checklist
|
TRANSMISSION_RATIO_LIMIT_ENABLED="true" | ||
TRANSMISSION_RATIO_LIMIT="0" | ||
TRANSMISSION_RPC_WHITELIST_ENABLED="false" | ||
TRANSMISSION_RPC_WHITELIST="127.0.0.1,192.168.*.*" |
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.
Shouldnt that include all private Networks (10.0.0.0/8 and 172.16.0.0/12)?
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 have to admit that I’m not a network expert, and I don’t know the specific details of these IP addresses. However, I think it depends on your network configuration. What I’m providing here is just a minimal configuration to access the main application on a standard local network.
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.
From my point of view, it may be unconventional, and it should be left to users as a custom configuration if they want more. But I may be wrong.
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 dont know either, but i have a 10.0.0.0/8 address range in my lan. would this work out of the box for me or not?
This is the spirit of the script. keep the configs minimal, but also it should run by default for anyone. Can you check that pls?
🛑 New scripts must first be submitted to ProxmoxVED for testing.
PRs for new scripts that skip this process will be closed.
✍️ Description
Create ct maker script
Create installation script
Create web json config file
Add version in version.json
=> Need to create header
🔗 Related PR / Issue
[Script request]: docker-transmission-openvpn => transmission-openvpn #7402
Link: # community-scripts/ProxmoxVE#7402
✅ Prerequisites (X in brackets)
🛠️ Type of Change (X in brackets)
README
,AppName.md
,CONTRIBUTING.md
, or other docs.🔍 Code & Security Review (X in brackets)
Code_Audit.md
&CONTRIBUTING.md
guidelinesAppName.sh
,AppName-install.sh
,AppName.json
)📋 Additional Information (optional)
I could not test with my fork because of hardcoded url referencing official ProxmoxVED.