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.

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

Deprecate 'keepends' argument in favor of 'drop' #2476

Merged
merged 4 commits into from
Dec 10, 2024
Merged

Deprecate 'keepends' argument in favor of 'drop' #2476

merged 4 commits into from
Dec 10, 2024

Conversation

MrQubo
Copy link
Contributor

@MrQubo MrQubo commented Sep 28, 2024

Add 'drop' argument in place of 'keepends' for tubes. 'keepends' still works but raises depreciation warning.

Resolves #2258.

@MrQubo MrQubo force-pushed the replace_keepends_with_drop branch 2 times, most recently from 88af217 to b9dca8c Compare September 28, 2024 21:35
@MrQubo MrQubo changed the title Deprecate 'keepends' in favor of 'drop' Deprecate 'keepends' argument in favor of 'drop' Sep 28, 2024
@MrQubo MrQubo force-pushed the replace_keepends_with_drop branch from b9dca8c to 2bede6a Compare September 28, 2024 21:47
@MrQubo MrQubo marked this pull request as draft September 28, 2024 21:47
@MrQubo MrQubo force-pushed the replace_keepends_with_drop branch from 2bede6a to 8d10734 Compare September 28, 2024 22:01
@MrQubo MrQubo marked this pull request as ready for review September 28, 2024 22:05
Arusekk
Arusekk requested changes Oct 3, 2024
View reviewed changes
pwnlib/tubes/tube.py Outdated Show resolved Hide resolved
pwnlib/tubes/tube.py Outdated Show resolved Hide resolved
pwnlib/tubes/tube.py Outdated Show resolved Hide resolved
pwnlib/tubes/tube.py Outdated Show resolved Hide resolved
@MrQubo MrQubo force-pushed the replace_keepends_with_drop branch 2 times, most recently from c9c6174 to 58ce74e Compare October 3, 2024 14:57
@MrQubo MrQubo requested a review from Arusekk October 3, 2024 17:25
@MrQubo MrQubo force-pushed the replace_keepends_with_drop branch from 832280c to 3e1225b Compare October 3, 2024 17:26
@MrQubo
Copy link
Contributor Author

MrQubo commented Oct 3, 2024

I have like 30 emails from failing pipeline in my inbox. xd

@MrQubo MrQubo force-pushed the replace_keepends_with_drop branch 2 times, most recently from d0ea491 to ddef6b3 Compare October 4, 2024 21:02
@peace-maker
Copy link
Member

When inserting the drop argument before the keepends one we'll break code using positional arguments like recvline(False). Can you switch it up so drop is added later?

It's annoying but we can switch the order when doing a major version bump. What do you think @Arusekk ?

@Arusekk
Copy link
Member

Arusekk commented Oct 24, 2024

@peace-maker I agree we should start working on Pwntools 5 soon, dropping py2, adding type annotations, and cleaning up dependencies, inconsistent interfaces and useless defaults.

@peace-maker peace-maker mentioned this pull request Oct 25, 2024
Open
8 tasks
@MrQubo
Copy link
Contributor Author

MrQubo commented Nov 1, 2024

@peace-maker I was thing of just removing keepends altogether.

@MrQubo MrQubo force-pushed the replace_keepends_with_drop branch from ddef6b3 to e4d6666 Compare November 1, 2024 01:27
MrQubo added 3 commits November 1, 2024 02:37
@MrQubo
Replace 'keepends' with 'drop'
7b951f1 
Add 'drop' argument in place of 'keepends' for tubes. 'keepends' still
works but raises depreciation warning.
@MrQubo
changelog
acdfe23
@MrQubo
Ignore line numbers in PyLint check
ee59abc
@MrQubo MrQubo force-pushed the replace_keepends_with_drop branch from e4d6666 to ee59abc Compare November 1, 2024 01:38
@MrQubo
Copy link
Contributor Author

MrQubo commented Nov 9, 2024

When inserting the drop argument before the keepends one we'll break code using positional arguments like recvline(False). Can you switch it up so drop is added later?

It's annoying but we can switch the order when doing a major version bump. What do you think @Arusekk ?

That was resolved.
Are we merging this?

@peace-maker peace-maker merged commit 74a300d into Gallopsled:dev Dec 10, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peace-maker peace-maker peace-maker approved these changes

@Arusekk Arusekk Awaiting requested review from Arusekk

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Merge drop and keepends kwargs
3 participants
@MrQubo @peace-maker @Arusekk