Samba: Add the ability to enable and disable trying to become a local master browser on a subnet #3950
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new configuration option, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ConfigFile as "config.yaml"
participant Doc as "Documentation/Translations"
participant TemplateEngine as "Template Engine (smb.gtpl)"
participant SambaService as "Samba Service"
User->>+ConfigFile: Set local_master option (true/false)
ConfigFile->>+TemplateEngine: Pass configuration value
TemplateEngine-->>-SambaService: Render smbd config with "local master" set accordingly
Doc-->>User: Updated docs & translation strings
SambaService->>SambaService: Initialize local master browser behavior based on setting
📜 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
samba/DOCS.md (1)
68-71: Clarify the option’s behavior in the documentation.
The current description is brief. Consider specifying that disabling the option will prevent the Samba server from attempting to become the local master browser, which improves clarity for the user.Suggested diff:
-### Option: `local_master` (required) -Enable to try and become a local master browser on a subnet. +### Option: `local_master` (required) +Enable or disable the Samba server's attempt to become the local master browser on the subnet.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
samba/CHANGELOG.md(1 hunks)samba/DOCS.md(2 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 (6)
`*/**(html|markdown|md)`: - For instructional content in doc...
*/**(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'
samba/CHANGELOG.mdsamba/config.yamlsamba/DOCS.md
`*/**(html|markdown|md)`: - Use bold to mark UI strings. - I...
*/**(html|markdown|md): - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
samba/CHANGELOG.mdsamba/config.yamlsamba/DOCS.md
`*/**(html|markdown|md)`: - Be brief in your replies and don...
*/**(html|markdown|md): - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
samba/CHANGELOG.mdsamba/config.yamlsamba/DOCS.md
`*/**(html|markdown|md)`: - Use sentence-style capitalizatio...
*/**(html|markdown|md): - Use sentence-style capitalization also in headings.
samba/CHANGELOG.mdsamba/config.yamlsamba/DOCS.md
`*/**(html|markdown|md)`: do not comment on HTML used for ic...
*/**(html|markdown|md): do not comment on HTML used for icons
samba/CHANGELOG.mdsamba/config.yamlsamba/DOCS.md
`*/**(html|markdown|md)`: Avoid flagging inline HTML for emb...
*/**(html|markdown|md): Avoid flagging inline HTML for embedding videos in future reviews for this repository.
samba/CHANGELOG.mdsamba/config.yamlsamba/DOCS.md
🔇 Additional comments (5)
samba/rootfs/usr/share/tempio/smb.gtpl (1)
5-5: LGTM!The
local mastersetting is correctly placed in the[global]section and properly uses the ternary operator to convert the boolean value to Samba's expected "yes"/"no" format.samba/translations/en.yaml (1)
14-16: LGTM!The translation entry follows the established pattern and provides a clear description of the feature's purpose.
samba/config.yaml (1)
30-30: LGTM!The configuration is well-structured with:
- Default value set to
trueto maintain backward compatibility- Correct boolean type definition in the schema
Also applies to: 57-57
samba/CHANGELOG.md (1)
2-5: LGTM!The changelog entry:
- Uses appropriate version increment for a new feature
- Clearly describes the added functionality
- Follows the established format
samba/DOCS.md (1)
39-39: Approved addition of thelocal_masterYAML configuration option.
The addition oflocal_master: truefollows the established pattern and clearly shows the default behavior.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
Oh, sorry, bumped the version in config.yaml also. |
Solution for the feature request #1716 and https://community.home-assistant.io/t/samba-smb-disable-master-browser/722039
Summary by CodeRabbit
New Features
Documentation
local_masterin the Samba add-on documentation, specifying its purpose and functionality.