Skip to content

Conversation

@gtsiam
Copy link

@gtsiam gtsiam commented Oct 24, 2025

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:

"gateway-address": "192.168.1.1"

Fixes #6457.


PS: With arch linux (clang 21.1.4):

./code_style.sh --check 2>&1 | wc -l
1634

I just did TR_SKIP_HOOKS=1 git commit.

@Neustradamus

This comment was marked as spam.

Copy link
Member

@ckerr ckerr left a 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.

@ckerr ckerr added needs code review needs notes PR title needs a one-sentence paragraph starting with 'Notes:' to summarize for the release notes type:feat A new feature labels Nov 10, 2025
@gtsiam gtsiam force-pushed the pr/gwaddr branch 3 times, most recently from ab2f704 to d2f982b Compare November 12, 2025 15:56
@gtsiam gtsiam requested a review from ckerr November 12, 2025 15:57
Copy link
Member

@ckerr ckerr left a 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?

@gtsiam
Copy link
Author

gtsiam commented Nov 12, 2025

Would be nice if code_style.sh --check actually worked and I could run it locally, but I don't feel like debugging it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs code review needs notes PR title needs a one-sentence paragraph starting with 'Notes:' to summarize for the release notes scope:core type:feat A new feature

Development

Successfully merging this pull request may close these issues.

natpmp should have an override for default gateway

4 participants