Skip to content

Conversation

@cdfmr
Copy link

@cdfmr cdfmr commented Dec 16, 2025

📦 Package Details

Maintainer: @

Description:
Add namesilo.com API to ddns-scripts package.


🧪 Run Testing Details

  • OpenWrt Version: r32320
  • OpenWrt Target/Subtarget: MediaTek/Filogic
  • OpenWrt Device: Cudy TR3000 v1

✅ Formalities

  • I have reviewed the CONTRIBUTING.md file for detailed contributing guidelines.

@GeorgeSapkin
Copy link
Member

You need to add a commit message, and use your real name both for author and sign-off, which is missing as per the contributing guidelines.

Copy link
Member

@GeorgeSapkin GeorgeSapkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And you need to bump the PKG_RELEASE in the Makefile.

Comment on lines 138 to 139


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

Comment on lines 594 to 595


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

@@ -0,0 +1,28 @@
#
#.Distributed under the terms of the GNU General Public License (GPL) version 2.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a SPDX license identifier instead.

}

# update subdomain record
rrid=`call_api dnsListRecords "domain=$domain" | get_rrid`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
rrid=`call_api dnsListRecords "domain=$domain" | get_rrid`
rrid=$(call_api dnsListRecords "domain=$domain" | get_rrid)

Comment on lines 24 to 26
call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL" | grep success && {
return 0
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean:

Suggested change
call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL" | grep success && {
return 0
}
call_api dnsUpdateRecord "domain=$domain&rrid=$rrid&rrhost=$username&rrvalue=$__IP&rrttl=$__TTL" | grep success && true

@cdfmr
Copy link
Author

cdfmr commented Dec 17, 2025

Updated with all suggestions.

@cdfmr cdfmr requested a review from GeorgeSapkin December 17, 2025 08:42
@GeorgeSapkin
Copy link
Member

You still need to add a commit message with a description of the changes, besides the subject.

@cdfmr cdfmr force-pushed the ddns-namesilo branch 3 times, most recently from 530a7ff to b142d07 Compare December 19, 2025 01:28
@@ -0,0 +1,24 @@
#
# SPDX-License-Identifier: GPL-2.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no such identifier. Did you mean GPL-2.0-only?

https://spdx.org/licenses/

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, should I update the file and re-commit?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please.

new service provider namesilo.com
config guide:
* set [domain] to apex domain
* set [username] to subdomain (without apex domain)
* set [password] to api key

Signed-off-by: Lin Fan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants