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

[docs] Add Resources folder with PCAPs and Useful Links sections #6778

Merged
merged 1 commit into from
May 14, 2021

Conversation

uri200
Copy link
Contributor

@uri200 uri200 commented May 5, 2021

Signed-off-by: Oriol Batalla obatalla@fb.com

Summary

Add a new section called Resources where to drop useful links, files, or information for Magma users.

Added PCAPs with GX/GY and inbound roaming examples

image

Test Plan

run docusaurus locally

Additional Information

  • This change is backwards-breaking

@uri200 uri200 requested review from a team and themarwhal May 5, 2021 22:51
@github-actions
Copy link
Contributor

github-actions bot commented May 5, 2021

Thanks for opening a PR! 💯

Howto

Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.

Checks. All required CI checks must pass before merge.

Merge. Once approved and passing all CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

All commits must be signed off. This is enforced by the DCO check.

Please take a moment to read through the Magma project's

If this is your first Magma PR, also consider reading

@uri200 uri200 requested a review from talkhasib May 5, 2021 22:51
Copy link
Contributor

@emakeev emakeev left a comment

Choose a reason for hiding this comment

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

This is a great idea, we are always searching for pcaps & protocol examples for unit tests & implementation specifics.


# PCAP collection

This is a collection of `.pcap` files that can be open using [Wireshark](https://www.wireshark.org/). Some of those PCAP collect data
Copy link
Member

Choose a reason for hiding this comment

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

Some of those PCAP collect data seems like an incomplete sentence :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch! it was hidden on my IDE far far in the right

Copy link
Contributor

@hcgatewood hcgatewood left a comment

Choose a reason for hiding this comment

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

Love the additions, thanks @uri200! Two requests

  • We're trying to keep the number of top-level sections as small as possible. For the pcaps etc, please put those into a subsection under Technical Reference. For the links, please put them somewhere in the intro/getting started section
  • Is the inline HTML necessary in the Markdown files? e.g. the <br />

@uri200
Copy link
Contributor Author

uri200 commented May 5, 2021

Love the additions, thanks @uri200! Two requests

  • We're trying to keep the number of top-level sections as small as possible. For the pcaps etc, please put those into a subsection under Technical Reference. For the links, please put them somewhere in the intro/getting started section
  • Is the inline HTML necessary in the Markdown files? e.g. the <br />

Hey @hcgatewood, initially I had it in Technical Resources, then I moved it to How Tos. After discussing with @talkhasib we thought it didn't belong to any of the categories displayed. I am fine with any location. Should we move Technical Reference and Usage under Resources?

For the <br /> the reason is I wanted to have the filename separated from the description. Adding that as a title didn't satisfied it since it was leaving a big gap. And no without the <br />, the same line was used for both link and description. See below the different options. I am open to chose between the current or any of those 2 if you like them better.

image

image

@hcgatewood
Copy link
Contributor

So the idea is to separate "Usage" as basic information for operators, and "Technical Reference" as more in-depth information for SIs, VARs, and devs. So I would say this info would fit best under Technical Reference > General subsection

For the Markdown -- just do something like this, maybe even as a list

@uri200
Copy link
Contributor Author

uri200 commented May 6, 2021

So the idea is to separate "Usage" as basic information for operators, and "Technical Reference" as more in-depth information for SIs, VARs, and devs. So I would say this info would fit best under Technical Reference > General subsection

For the Markdown -- just do something like this, maybe even as a list

Techincal Reference/General sounds good to me if @talkhasib doesn't have any problem with it.

@uri200
Copy link
Contributor Author

uri200 commented May 6, 2021

Found a better way for the entries
image

@uri200 uri200 force-pushed the docusaurus_pcap_1 branch 2 times, most recently from a231485 to ed66801 Compare May 6, 2021 00:23
@talkhasib
Copy link
Contributor

under Reference/General sounds good

@uri200
Copy link
Contributor Author

uri200 commented May 6, 2021

Done, added it to Technical Reference section. See picture on the description of the PR

Copy link
Contributor

@hcgatewood hcgatewood left a comment

Choose a reason for hiding this comment

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

Thanks for adding. Couple small comments before merge

hide_title: true
---

*Last Updated: 4/20/2021*
Copy link
Contributor

Choose a reason for hiding this comment

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

For both files: these timestamps aren't necessary / won't get updated when people make changes. Docusaurus has a feature to add these automatically, so look into that if you're interested in providing this affordance

"type": "subcategory",
"label": "General",
"ids": [
"resources/pcap",
Copy link
Contributor

Choose a reason for hiding this comment

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

For both ids: prefix them both with either dev_ or ref_ for namespacing purposes


This is a list of links that may be useful to understand Magma

- [Netmanias](https://www.netmanias.com/)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add some minor info about what these links contain

@uri200 uri200 enabled auto-merge (squash) May 12, 2021 17:48
Signed-off-by: Oriol Batalla <obatalla@fb.com>
@uri200 uri200 requested a review from a team May 14, 2021 04:16
@uri200 uri200 merged commit f23a496 into magma:master May 14, 2021
@codecov
Copy link

codecov bot commented May 14, 2021

Codecov Report

Merging #6778 (83a20fb) into master (4f45a0d) will decrease coverage by 2.47%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6778      +/-   ##
==========================================
- Coverage   64.45%   61.97%   -2.48%     
==========================================
  Files         671      521     -150     
  Lines       46527    35696   -10831     
  Branches     1318        0    -1318     
==========================================
- Hits        29990    22124    -7866     
+ Misses      13120    10473    -2647     
+ Partials     3417     3099     -318     
Flag Coverage Δ
cloud_lint 65.48% <ø> (+0.01%) ⬆️
feg-lint 56.55% <ø> (+0.01%) ⬆️
lte-test ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...way/python/magma/enodebd/state_machines/enb_acs.py
lte/gateway/python/magma/enodebd/stats_manager.py
...ay/python/magma/magmad/check/network_check/ping.py
...ateway/python/magma/mobilityd/ip_allocator_pool.py
...agma/enodebd/device_config/enodeb_configuration.py
lte/gateway/python/magma/mobilityd/mac.py
...subscriberdb/protocols/diameter/application/s6a.py
...python/magma/policydb/servicers/policy_servicer.py
.../gateway/python/magma/subscriberdb/rpc_servicer.py
...gateway/python/magma/common/serialization_utils.py
... and 142 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f45a0d...83a20fb. Read the comment docs.

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.

5 participants