-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: Add NAT-PMP gateway_address configuration option.
#7735
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?
Conversation
This comment was marked as spam.
This comment was marked as spam.
ckerr
left a comment
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 don't have any strong feelings about the feature & would appreciate another opinion e.g. from other contributors about the feature itself.
Implementation basically LGTM. I have some nit suggestions but nothing serious. Requesting changes for the nits.
ab2f704 to
d2f982b
Compare
ckerr
left a comment
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.
Implementation LGTM.
I'd still appreciate feedback from other @transmission/contributors about the feature itself. Is this something we should use?
|
Would be nice if |
My C++ skills are rusty (pun intended) and this isn't an easy code base to navigate. Nevertheless, this does the job.
With this change, you can set the gateway address used by libnatpmp in
settings.json:Fixes #6457.
PS: With arch linux (clang 21.1.4):
I just did
TR_SKIP_HOOKS=1 git commit.