-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[SVH] Add WINS Server and MasterBrowser options to SAMBA AddOn #3894
base: master
Are you sure you want to change the base?
[SVH] Add WINS Server and MasterBrowser options to SAMBA AddOn #3894
Conversation
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.
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
📝 WalkthroughWalkthroughThe pull request introduces a new version entry for 12.4.1 in the Samba add-on, which includes two significant features: the ability to enable SAMBA as a local browser master and the option to enable the SAMBA WINS Server. These changes are reflected across multiple files, including the changelog, documentation, configuration schema, template, and translations, enhancing the configurability of the Samba service without affecting existing functionalities. Changes
Sequence DiagramsequenceDiagram
participant User
participant SambaConfig
participant SambaService
User->>SambaConfig: Set winsserver_mode and masterbrowser_mode
SambaConfig->>SambaService: Apply network configuration
alt WINS Server Enabled
SambaService->>SambaService: Enable WINS support
end
alt Master Browser Enabled
SambaService->>SambaService: Configure as local master browser
end
SambaService-->>User: Network services configured
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
samba/config.yaml (1)
Line range hint
2-2
: Update version number to match changelog.The version number should be updated to 12.4.1 to match the changelog.
-version: 12.4.0 +version: 12.4.1
🧹 Nitpick comments (1)
samba/translations/en.yaml (1)
31-34
: Enhance WINS Server description for clarity.The current description could be more informative about the purpose of WINS.
- Enable this to use old legacy WINS protocols on this Samba add-on. + Enable this to use WINS (Windows Internet Name Service) for NetBIOS name resolution. This allows the Samba server to act as a WINS server for the network.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
samba/CHANGELOG.md
(1 hunks)samba/DOCS.md
(1 hunks)samba/config.yaml
(2 hunks)samba/rootfs/usr/share/tempio/smb.gtpl
(1 hunks)samba/translations/en.yaml
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
samba/DOCS.md (6)
Pattern */**(html|markdown|md)
: - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
Pattern */**(html|markdown|md)
: - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
Pattern */**(html|markdown|md)
: - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Pattern */**(html|markdown|md)
: - Use sentence-style capitalization also in headings.
Pattern */**(html|markdown|md)
: do not comment on HTML used for icons
Pattern */**(html|markdown|md)
: Avoid flagging inline HTML for embedding videos in future reviews for this repository.
samba/CHANGELOG.md (6)
Pattern */**(html|markdown|md)
: - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
Pattern */**(html|markdown|md)
: - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
Pattern */**(html|markdown|md)
: - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Pattern */**(html|markdown|md)
: - Use sentence-style capitalization also in headings.
Pattern */**(html|markdown|md)
: do not comment on HTML used for icons
Pattern */**(html|markdown|md)
: Avoid flagging inline HTML for embedding videos in future reviews for this repository.
samba/config.yaml (6)
Pattern */**(html|markdown|md)
: - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
Pattern */**(html|markdown|md)
: - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
Pattern */**(html|markdown|md)
: - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Pattern */**(html|markdown|md)
: - Use sentence-style capitalization also in headings.
Pattern */**(html|markdown|md)
: do not comment on HTML used for icons
Pattern */**(html|markdown|md)
: Avoid flagging inline HTML for embedding videos in future reviews for this repository.
🔇 Additional comments (5)
samba/rootfs/usr/share/tempio/smb.gtpl (2)
6-8
: LGTM: WINS server configuration is correctly implemented.The WINS server configuration is properly implemented using the standard Samba parameter.
10-15
: LGTM: Master Browser configuration follows best practices.The Master Browser configuration includes all necessary settings:
- domain master
- preferred master
- local master
- os level (set to 255)
These settings ensure the server will effectively function as the Master Browser.
samba/config.yaml (1)
39-40
: LGTM: New options are well-defined with appropriate defaults.The new options are properly configured with appropriate default values and correct schema definitions.
Also applies to: 61-62
samba/CHANGELOG.md (1)
2-6
: LGTM: Changelog entries are clear and follow the format.The changelog properly documents the new features with clear descriptions.
samba/DOCS.md (1)
Line range hint
1-24
: Documentation structure and style look good!The documentation follows the style guidelines with:
- Clear step-by-step instructions
- Proper use of bold for UI elements
- Direct and authoritative tone
@coderabbitai review |
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.
Thank you for your PR!
Can you elaborate a bit more what your use case is/what you try to solve adding these options?
## 12.4.1 | ||
|
||
- Add the option to enable SAMBA to become local browsermaster | ||
- Add the option to enable SAMBA WINS Server |
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 https://learn.microsoft.com/en-us/windows-server/networking/technologies/wins/wins-top:
Windows Internet Name Service (WINS) is a legacy computer name registration and resolution service that maps computer NetBIOS names to IP addresses.
If you do not already have WINS deployed on your network, do not deploy WINS - instead, deploy Domain Name System (DNS). [...]
Since even Microsoft considers WINS legacy, I don't think we should add support for it on our end.
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.
MasterBrowser in my network helps with navigating my servers/shares.
wins is old, can live without.
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.
Thanks at least for considering
Add configuration options to the SAMBA AddOn to enable:
Summary by CodeRabbit
Release Notes for Samba Add-on v12.4.1
New Features
Configuration Updates
winsserver_mode
(default: false) andmasterbrowser_mode
(default: true).These updates provide users with increased control over Samba's network interaction capabilities while preserving existing functionality.