Skip to content

Centralize P4 learning materials in README (#43) #45

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Centralize P4 learning materials in README (#43) #45

wants to merge 2 commits into from

Conversation

@koushikcs562
Copy link

@koushikcs562 koushikcs562 commented Nov 18, 2025

This PR addresses issue #43 by moving the P4 learning materials from contributor_guidance.md into the main README under a new "Learning Resources" section. This improves visibility and simplifies onboarding.

@koushikcs562 koushikcs562 force-pushed the centralize-learning-materials branch from 268c73e to 53b06ae Compare November 18, 2025 06:55
fruffy
fruffy reviewed Nov 19, 2025
View reviewed changes
Copy link
Collaborator

@fruffy fruffy left a comment

Choose a reason for hiding this comment

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

Can you rebase and also sign your commit with DCO?
https://github.com/p4lang/p4c/blob/main/CONTRIBUTING.md#contributing-license

.github/instructions/snyk_rules.instructions.md Outdated
@@ -0,0 +1,14 @@
---
Copy link
Collaborator

@fruffy fruffy Nov 19, 2025

Choose a reason for hiding this comment

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

This is unrelated.

@koushikcs562 koushikcs562 force-pushed the centralize-learning-materials branch 2 times, most recently from bad6403 to 09c215d Compare November 19, 2025 06:03
@qobilidop
Copy link
Member

qobilidop commented Nov 21, 2025

This is already completed by @Dscano in #42.

@qobilidop qobilidop closed this Nov 21, 2025
@koushikcs562
Copy link
Author

koushikcs562 commented Nov 21, 2025

@fruffy can you once check my pr please #45

@fruffy fruffy reopened this Nov 21, 2025
@fruffy
Copy link
Collaborator

fruffy commented Nov 21, 2025

This is already completed by @Dscano in #42.

Davide didn't add the learning materials to the README and instead created a folder. Having a section on the top-level is actually a fine approach. It makes the README we have currently look less barren

@fruffy
Copy link
Collaborator

fruffy commented Nov 21, 2025

@fruffy can you once check my pr please #45

You need to resolve the conflicts, sign the DCO, and reduce the duplication that was created.

fruffy
fruffy reviewed Nov 21, 2025
View reviewed changes
README.md Outdated
- https://github.com/p4lang/p4c/blob/main/docs/compiler-design.pdf
- Introduction to P4Runtime: [Next-Gen SDN Tutorial - Session 1: P4 and P4Runtime Basics](https://www.youtube.com/watch?v=KRx92qSLgo4)

> These resources were previously located in `contributor_guidance.md` and are now centralized here for easier access.
Copy link
Collaborator

@fruffy fruffy Nov 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Remove this line

@qobilidop
Copy link
Member

qobilidop commented Nov 21, 2025

Davide didn't add the learning materials to the README and instead created a folder. Having a section on the top-level is actually a fine approach. It makes the README we have currently look less barren

@fruffy Makes sense. In this case, @koushikcs562 could you remove the file (materials/learning_materials.md) that @Dscano added?

@koushikcs562
Copy link
Author

koushikcs562 commented Nov 22, 2025

@qobilidop yes I'll remove it thank you for your feedback

@Dscano
Copy link
Collaborator

Dscano commented Nov 28, 2025

@qobilidop yes I'll remove it thank you for your feedback

@koushikcs562, Can you please sign your comments? Here is a guide for signing: https://www.secondstate.io/articles/dco.

@koushikcs562
Copy link
Author

koushikcs562 commented Nov 28, 2025

@Dscano thank you for your patience I am trying to do that and already did but I think it went wrong I ll check and update you shortly I am taking time because it is my first pull request thank you for being patient to everyone

@koushikcs562 koushikcs562 force-pushed the centralize-learning-materials branch 3 times, most recently from e981b12 to 4a57f91 Compare December 2, 2025 06:05
@koushikcs562
Copy link
Author

koushikcs562 commented Dec 2, 2025

Hi @fruffy @qobilidop @Dscano — I’ve resolved the README conflict, removed the duplicate file, and cleaned up the accidental submodule. All commits are signed off and checks are green. The PR should now be ready for review and merge. Thanks a lot for your guidance throughout this process!

Dscano
Dscano requested changes Dec 2, 2025
View reviewed changes
Dscano
Dscano requested changes Dec 2, 2025
View reviewed changes
@Dscano
Copy link
Collaborator

Dscano commented Dec 2, 2025

@koushikcs562 Thank you for the support. Once you have addressed the reviewer's comments, could you please squash the commits?

Dscano
Dscano approved these changes Dec 2, 2025
View reviewed changes
Copy link
Collaborator

@Dscano Dscano left a comment

Choose a reason for hiding this comment

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

LGTM, please sign your comment.

@koushikcs562
Copy link
Author

koushikcs562 commented Dec 2, 2025

@Dscano i ll do it by today night thank you

@koushikcs562 koushikcs562 force-pushed the centralize-learning-materials branch 2 times, most recently from fe48fe3 to 886ab21 Compare December 2, 2025 09:58
@koushikcs562
Copy link
Author

koushikcs562 commented Dec 2, 2025

Hi @fruffy @qobilidop @Dscano — I’ve squashed the commits into one clean commit with proper DCO sign-off. The branch is updated and ready for merge. Thanks again for your guidance!
Signed-off-by: Cs Koushik [email protected]

@Dscano
Copy link
Collaborator

Dscano commented Dec 2, 2025

Hi @fruffy @qobilidop @Dscano — I’ve squashed the commits into one clean commit with proper DCO sign-off. The branch is updated and ready for merge. Thanks again for your guidance! Signed-off-by: Cs Koushik [email protected]

Sorry, I didn’t express myself properly . What I meant to say is that the DCO check is not passing. Could you please fix it?

@koushikcs562 koushikcs562 force-pushed the centralize-learning-materials branch 2 times, most recently from 62ba360 to f9ea1ce Compare December 2, 2025 13:17
@koushikcs562 koushikcs562 force-pushed the centralize-learning-materials branch from 49e14c0 to 235b3b6 Compare December 2, 2025 13:23
@Dscano
Copy link
Collaborator

Dscano commented Dec 2, 2025

@koushikcs562 Could you please remove the learning_materials.md file, which is located inside the materials folder?
You moved the content of this file into the "main" README .

@koushikcs562 koushikcs562 deleted the centralize-learning-materials branch December 2, 2025 13:59
@koushikcs562 koushikcs562 restored the centralize-learning-materials branch December 2, 2025 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fruffy fruffy fruffy left review comments

@Dscano Dscano Dscano approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@koushikcs562 @qobilidop @fruffy @Dscano