Skip to content

Update README with the help of Copilot #1895

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

Open
wants to merge 17 commits into
base: dev
Choose a base branch
from

Conversation

egecetin
Copy link
Collaborator

@egecetin egecetin commented Jul 25, 2025

Created with Copilot's Claude Sonnet 4 model and some minor human corrections :)

I asked copilot to update README file to make it visually appealing. To be honest I'm impressed with the result. It is looks like modern and attractive. Let me know what you think? @seladb @clementperon @Dimi1010 @tigercosmos

If anyone is currently using the library or considering using it and sees this PR/comment, please share your thoughts also

@Dimi1010
Copy link
Collaborator

Looks good. A bit too happy on starting sensences with emotes, tho. The ones in the comments in cpp example code got to go.

Really liking the table structures.

Copy link

codecov bot commented Jul 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.81%. Comparing base (6d8b747) to head (bd718e6).
⚠️ Report is 9 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1895      +/-   ##
==========================================
- Coverage   82.85%   82.81%   -0.04%     
==========================================
  Files         291      291              
  Lines       51565    51592      +27     
  Branches    11162    11208      +46     
==========================================
+ Hits        42723    42725       +2     
- Misses       7646     7701      +55     
+ Partials     1196     1166      -30     
Flag Coverage Δ
alpine320 74.59% <ø> (-0.01%) ⬇️
fedora42 74.71% <ø> (-0.02%) ⬇️
macos-13 81.03% <ø> (-0.04%) ⬇️
macos-14 81.03% <ø> (-0.04%) ⬇️
macos-15 81.03% <ø> (-0.04%) ⬇️
mingw32 70.00% <ø> (+0.09%) ⬆️
mingw64 69.99% <ø> (+0.09%) ⬆️
npcap ?
rhel94 74.44% <ø> (-0.04%) ⬇️
ubuntu2004 58.74% <ø> (-0.03%) ⬇️
ubuntu2004-zstd 58.83% <ø> (-0.03%) ⬇️
ubuntu2204 74.38% <ø> (+<0.01%) ⬆️
ubuntu2204-icpx 60.74% <ø> (-0.08%) ⬇️
ubuntu2404 74.61% <ø> (-0.02%) ⬇️
ubuntu2404-arm64 74.60% <ø> (-0.02%) ⬇️
unittest 82.81% <ø> (-0.04%) ⬇️
windows-2022 84.95% <ø> (+0.16%) ⬆️
windows-2025 84.97% <ø> (+0.13%) ⬆️
winpcap 84.97% <ø> (+0.03%) ⬆️
xdp 51.28% <ø> (-0.02%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

</picture>,
__Android__
<source media="(prefers-color-scheme: dark)" srcset="https://github.com/PcapPlusPlus/pcapplusplus.github.io/raw/master/static/img/os-logos/logo-apple-dark.png"/>
<img src="https://github.com/PcapPlusPlus/pcapplusplus.github.io/raw/master/static/img/os-logos/logo-apple.png" alt="macOS" width="48" height="48"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should remove the Apple Logo here as there is no Windows, Linux Android ,etc Logo

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually there are other logos but for some reason they are not properly download/render from time to time. Refreshing the page solves it. I don't know why

@tigercosmos
Copy link
Collaborator

Please also put the screenshot of the new file.

@Dimi1010
Copy link
Collaborator

@tigercosmos you can check the rich diff, it visualizes the changes.

image

@tigercosmos
Copy link
Collaborator

@Dimi1010 Thanks, I didn't know that.

@egecetin
Copy link
Collaborator Author

egecetin commented Jul 25, 2025

@tigercosmos Or you can check my forked repo https://github.com/egecetin/PcapPlusPlus/tree/improve-readme it also renders it

Copy link
Collaborator

@tigercosmos tigercosmos left a comment

Choose a reason for hiding this comment

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

Looks more fancy

@seladb
Copy link
Owner

seladb commented Jul 27, 2025

I think this version of the README has too many emojis and icons. It's also very "noisy" and has way too much details. We can probably take some of it, but I would make it simpler

@egecetin egecetin marked this pull request as ready for review July 31, 2025 06:10
@egecetin egecetin requested a review from seladb as a code owner July 31, 2025 06:10
@seladb
Copy link
Owner

seladb commented Aug 6, 2025

@egecetin I think Copilot has some good ideas about the design, but I'd take what we have now and improve it instead of letting it revise the entire file:

  • I think this section is redundant: image

  • Table of Contents -> About PcapPlusPlus is redundant

  • I don't like the sub-title in each section that is centered, for example: image

  • Feature Overview - contains too much text and details, I'd revert it to what we currently have

  • Packet Capture Backends - can be included in Feature Overview like today (maybe in a fancier way?)

    • PF_RING and eBPF XDP are missing
    • Remote Capture is probably not very important
  • API Documentation - the badges seem redundant, I like our current version more

  • I like our current Multi Platform Support better with icons instead of emojis, and less text

  • High-Performance Packet Processing Support

    • Redundant badges
    • Maybe shouldn't be in columns?
  • Contributing - too much text, could be simplified

  • License - too much text, could be simplified

@egecetin
Copy link
Collaborator Author

egecetin commented Aug 6, 2025

@seladb Sure I'll work on it

@tigercosmos
Copy link
Collaborator

@egecetin, one reminder: please also update the translations.

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