Skip to content

Scapy 2.4.0: Little but obvious code improvements#1226

Merged
guedou merged 1 commit intosecdev:masterfrom
karpierz:little_but_obvious_code_improvements
Mar 9, 2018
Merged

Scapy 2.4.0: Little but obvious code improvements#1226
guedou merged 1 commit intosecdev:masterfrom
karpierz:little_but_obvious_code_improvements

Conversation

@karpierz
Copy link
Copy Markdown
Contributor

@karpierz karpierz commented Mar 8, 2018

Scapy Version: 2.4.0rc5-13
System: Windows7/Windows10
Python Version: 2.7.14

Copy link
Copy Markdown
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.

Very nice PR ! A few minor comments

Comment thread scapy/base_classes.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can even unify those lines

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep. I know, but did not want to change sources to deeply :)
Of course I will change it in next commit.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread scapy/fields.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Idem

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread scapy/packet.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Idem

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread scapy/sendrecv.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In fact, setting timeout to -1 is useless.
Those changes have been removed in #1142

To avoid conflicts, please do not edit it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@codecov-io
Copy link
Copy Markdown

codecov-io commented Mar 8, 2018

Codecov Report

Merging #1226 into master will increase coverage by 0.02%.
The diff coverage is 68.75%.

@@            Coverage Diff             @@
##           master    #1226      +/-   ##
==========================================
+ Coverage   84.75%   84.78%   +0.02%     
==========================================
  Files         159      159              
  Lines       38247    38210      -37     
==========================================
- Hits        32417    32396      -21     
+ Misses       5830     5814      -16
Impacted Files Coverage Δ
scapy/layers/bluetooth.py 85.95% <0%> (ø) ⬆️
scapy/layers/tls/keyexchange.py 83.22% <0%> (+0.17%) ⬆️
scapy/layers/inet.py 69.97% <0%> (+0.05%) ⬆️
scapy/layers/tls/handshake_sslv2.py 91.95% <100%> (-0.1%) ⬇️
scapy/base_classes.py 87.16% <100%> (+0.8%) ⬆️
scapy/dadict.py 98.66% <100%> (-0.06%) ⬇️
scapy/fields.py 90.32% <100%> (-0.04%) ⬇️
scapy/packet.py 76.31% <60%> (+0.06%) ⬆️
scapy/asn1/ber.py 82.28% <0%> (+0.28%) ⬆️
scapy/layers/tls/record.py 92.08% <0%> (+0.87%) ⬆️
... and 1 more

@karpierz karpierz mentioned this pull request Mar 9, 2018
@karpierz karpierz closed this Mar 9, 2018
@karpierz karpierz force-pushed the little_but_obvious_code_improvements branch from b19ef99 to 617920f Compare March 9, 2018 11:44
@karpierz karpierz reopened this Mar 9, 2018
@karpierz karpierz closed this Mar 9, 2018
@karpierz karpierz reopened this Mar 9, 2018
@karpierz
Copy link
Copy Markdown
Contributor Author

karpierz commented Mar 9, 2018

PR is ready for merge.

@guedou guedou merged commit 865300d into secdev:master Mar 9, 2018
@guedou
Copy link
Copy Markdown
Member

guedou commented Mar 9, 2018

Thanks for this PR!

@karpierz karpierz deleted the little_but_obvious_code_improvements branch March 9, 2018 18:38
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.

4 participants