Skip to content
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

Update discovery.md on mDNS Information #786

Merged
merged 6 commits into from
Nov 7, 2024
Merged

Conversation

axkar
Copy link
Collaborator

@axkar axkar commented Oct 30, 2024

Add information about:

Description

Checklist

Tick relevant boxes, this PR is-a or has-a:

  • Bugfix
    • Regression tests
    • ChangeLog updates (for next release)
  • Feature
    • YANG model change => revision updated?
    • Regression tests added?
    • ChangeLog updates (for next release)
    • Documentation added?
  • Test changes
    • Checked in changed Readme.adoc (make test-spec)
    • Added new test to group Readme.adoc and yaml file
  • Code style update (formatting, renaming)
  • Refactoring (please detail in commit messages)
  • Build related changes
  • Documentation content changes
    • ChangeLog updated (for major changes)
  • Other (please describe):

Copy link
Contributor

@troglobit troglobit left a comment

Choose a reason for hiding this comment

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

A few comments I'd like you to address before we merge this.

First, I'd really like to see the netbrowse section at the very beginning of this document. The document will come to life quicker with the nice picture of of the browser showing a listing of multiple devices.

Additionally, I'd like you to overhaul the whole page to not use the default Qemu MAC address (00-00-00) and instead use something else, e.g., c0-ff-ee or similar that we use elsewhere. Check with J-O for good examples.

Also, a representative example for netbrowse would be to have more than one device in the listing, at least one more, and use a different hostname than the default -- because that's one of the first things an administrator changes. Again, check with J-O for good name examples, we use descriptive names in the tests so should we here in the docs.

doc/discovery.md Outdated Show resolved Hide resolved
doc/discovery.md Outdated Show resolved Hide resolved
doc/discovery.md Outdated Show resolved Hide resolved
doc/discovery.md Outdated Show resolved Hide resolved
doc/discovery.md Outdated Show resolved Hide resolved
doc/discovery.md Outdated Show resolved Hide resolved
@jovatn
Copy link
Contributor

jovatn commented Nov 6, 2024

Plenty of commits. :-/
Anyhow, I hope it is now in shape for a new review.

axkar and others added 6 commits November 7, 2024 09:00
Add information about:
  - mDNS alias
  - https://network.local

[skip ci]

Fixes #738
Use infix-c0-ff-ee (or similar) rather than infix-00-00-00
Adapting picture and text for netbrowse function to use more than on unit

[skip ci]
- Adding info on how to disable LLDP
- Avoiding clickable URL to https://network.local

[skip ci]
Changing title on the section on netbrowse service

[skip ci]
Use proposal from review.

[skip ci]
Copy link
Contributor

@jovatn jovatn left a comment

Choose a reason for hiding this comment

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

LGTM. Additional documentation on netbrowse can be added later.

Copy link
Contributor

@troglobit troglobit left a comment

Choose a reason for hiding this comment

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

Amazing teamwork @axkar and @jovatn, I like it a lot!

@troglobit troglobit merged commit 7a9f1c1 into main Nov 7, 2024
@troglobit troglobit deleted the update-discovery-md branch November 7, 2024 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants