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

bluetooth: Add EIR appearance #4610

Merged
merged 1 commit into from
Dec 22, 2024

Conversation

antoniovazquezblanco
Copy link
Contributor

Just another Bluetooth EIR type :)

Checklist:

  • If you are new to Scapy: I have checked CONTRIBUTING.md (esp. section submitting-pull-requests)
  • I squashed commits belonging together
  • I added unit tests or explained why they are not relevant
  • I executed the regression tests (using cd test && ./run_tests or tox)
  • If the PR is still not finished, please create a Draft Pull Request

Copy link

codecov bot commented Dec 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.61%. Comparing base (f793b27) to head (5120092).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4610   +/-   ##
=======================================
  Coverage   81.60%   81.61%           
=======================================
  Files         359      359           
  Lines       86033    86040    +7     
=======================================
+ Hits        70210    70219    +9     
+ Misses      15823    15821    -2     
Files with missing lines Coverage Δ
scapy/layers/bluetooth.py 90.29% <100.00%> (+0.06%) ⬆️

... and 9 files with indirect coverage changes

@XenoKovah
Copy link

Once again I just want to support this PR and say that I'd find this capability valuable for Bluetooth parsing via Scapy.

@gpotter2
Copy link
Member

gpotter2 commented Dec 22, 2024

Excuse my paranoia.. but this kind of duplicated comments on multiple PRs make me wonder: is this another JiaT75? That's a total of 4 PRs on bluetooth with someone commenting they like it ^^'

#4602
#4603
#4610
#4612

PR looks fine, but for a second I was wondering if there was a backdoor in the unit tests.

@XenoKovah
Copy link

I'm not a very weird or mysterious user. I have a very easily googleable real name which is the same as this account name which is very attributable to my interest in Bluetooth. And Antonio (who's name is also very easily tied to a real identity through simple googling) is doing the changes rather than me, to help me out because he's contributed to Scapy before and knows your regression testing methodology which I don't.

@gpotter2
Copy link
Member

I must apologise, that was rude. Sorry about that I tried to fix my comment but it's a bit late.

Thank you both for your interest in Scapy. Those PRs both look good, we'll get to them very soon.

gpotter2
gpotter2 previously approved these changes Dec 22, 2024
Copy link
Member

@gpotter2 gpotter2 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 the PR & sorry again.

@gpotter2
Copy link
Member

The 2 PRs conflicted so I took the liberty to rebase against master, fixing the conflicts.

gpotter2
gpotter2 previously approved these changes Dec 22, 2024
@gpotter2 gpotter2 enabled auto-merge (squash) December 22, 2024 21:59
@antoniovazquezblanco
Copy link
Contributor Author

Nothing to worry about. From my side, I tried to keep it in small changes to make it easy to review and also because I do not have big slots of time lately. That is also a reason why copy paste seemed to be a little bit faster than writting custom messages for each PR.

I personally think it is a good thing you were a little bit paranoid. That speaks good of the process review you carry out. Thanks for the involvement and do not apologise for that please. If you need anything to verify my identity I am at your disposal.

Thank you again! :)

@gpotter2
Copy link
Member

My initial (before the edit) comment was inappropriate, so there's really no need to thank me.

I'm fine with very succinct descriptions of PRs when the content is very clear and tests are included, which is always the case here. You don't need to verify anything don't worry :)

Anyways, Merry Christmas to you all.

@gpotter2 gpotter2 merged commit eb279cc into secdev:master Dec 22, 2024
23 checks passed
@antoniovazquezblanco antoniovazquezblanco deleted the appearance branch December 23, 2024 08:59
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