Skip to content

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):

@axkar axkar requested review from jovatn and troglobit October 30, 2024 14:36
@axkar axkar force-pushed the update-discovery-md branch from 67da91d to f520721 Compare October 30, 2024 15:46
@troglobit troglobit self-assigned this Oct 31, 2024
@axkar axkar force-pushed the update-discovery-md branch from f520721 to 49d2e99 Compare November 1, 2024 09:42
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.

@jovatn
Copy link
Contributor

jovatn commented Nov 6, 2024

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

Ahmed Karic 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]
@jovatn jovatn force-pushed the update-discovery-md branch from c887d41 to d277f5a Compare November 7, 2024 08:07
@jovatn jovatn requested a review from troglobit November 7, 2024 08:08
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