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

OSRB review #55

Merged
merged 5 commits into from
Sep 2, 2022
Merged

OSRB review #55

merged 5 commits into from
Sep 2, 2022

Conversation

mzbroch
Copy link
Contributor

@mzbroch mzbroch commented Aug 11, 2022

No description provided.

@progala
Copy link
Contributor

progala commented Aug 12, 2022

I'll commit suggestions and updates once I've got my rights fixed.

For now, here are some generic comments:

Some, but not all, models have attributes listed in a list. We should update all model descriptions to follow the same format. Example format with attributes enumerated is here:

https://github.com/nautobot/nautobot-plugin-firewall-models/blob/develop/docs/models/policy.md

- It'd be good to include a section with an example illustrating the order in which objects should be created. Example of how we did it for the FW models:

https://github.com/nautobot/nautobot-plugin-firewall-models/blob/develop/docs/models.md#creation-order

- There is no mention of where in GUI these models are exposed.

- The name of the models don't always match the names in the GUI. It might be helpful to include a table with the mappings. Alternatively include the GUI name in the description of each model.

- Extra Attributes section should be moved above models referring to it. Alternatively, provide a link in the model descriptions.

- Split README.md into multiple files, each containing logical sections. Place the individual files in /docs and include MKDocs mkdocs.yml file in the root directory. This file will define the docs config used for deploying documentation to Read The Docs.

Example mkdocs.yml file:

https://github.com/nautobot/nautobot-plugin-firewall-models/blob/develop/mkdocs.yml

README.md Outdated Show resolved Hide resolved
@whitej6
Copy link
Contributor

whitej6 commented Aug 25, 2022

@progala & @mzbroch Please review #57 This was only changing where docs live and minor MD fixes. The content stayed the same.

@mzbroch mzbroch force-pushed the pr-docs-review branch 2 times, most recently from 1eac009 to 91e0125 Compare August 26, 2022 05:47
@mzbroch
Copy link
Contributor Author

mzbroch commented Aug 26, 2022

DONE:

  • Updates as per comments
  • Moved extra attributes
  • Mkdocs

TODO:

  • Provide common documentation format for Models. Indicate where the models are exposed in the ** Indicate how models are named in the UI. (provide model name vs verbose/UI name mapping. UI.** mzb: I like the documentation format, but as per my knowledge it has not been a standard. For now I am not going to rewrite it unless we have a common standard for it.
  • Provide stepped instructions to follow how to add a new Peering : internal and external, focus on creation-order (tracked in Documentation Improvements #58 )

@mzbroch mzbroch changed the title [WIP] Przemek's OSRB review OSRB review Aug 26, 2022
@mzbroch
Copy link
Contributor Author

mzbroch commented Aug 26, 2022

@progala Please review it again. Comments have been addressed.

docs/cisco_use_case.md Outdated Show resolved Hide resolved
docs/example.md Outdated Show resolved Hide resolved
docs/example.md Outdated Show resolved Hide resolved
docs/juniper_use_case.md Outdated Show resolved Hide resolved
docs/models.md Outdated Show resolved Hide resolved
docs/models.md Outdated Show resolved Hide resolved
docs/models.md Outdated Show resolved Hide resolved
docs/models.md Outdated Show resolved Hide resolved
docs/models.md Outdated Show resolved Hide resolved
docs/models.md Outdated Show resolved Hide resolved
Co-authored-by: Przemek Rogala <[email protected]>
@mzbroch
Copy link
Contributor Author

mzbroch commented Aug 31, 2022

@progala Added the first iteration of manual on how to create bgp peerings

@mzbroch mzbroch requested a review from progala September 1, 2022 07:34
docs/manual.md Outdated Show resolved Hide resolved
docs/manual.md Show resolved Hide resolved
docs/manual.md Outdated Show resolved Hide resolved
docs/manual.md Outdated Show resolved Hide resolved
docs/manual.md Outdated Show resolved Hide resolved
docs/manual.md Outdated Show resolved Hide resolved
docs/manual.md Outdated Show resolved Hide resolved
docs/manual.md Outdated Show resolved Hide resolved
docs/manual.md Outdated Show resolved Hide resolved
docs/manual.md Outdated Show resolved Hide resolved
mkdocs.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@progala progala left a comment

Choose a reason for hiding this comment

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

I made updates to image metadata and converted some plain URLs to markdown.

Everything else looks good.

@mzbroch mzbroch merged commit 3e44267 into main Sep 2, 2022
@whitej6 whitej6 deleted the pr-docs-review branch March 14, 2023 21:46
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