Skip to content

Return arg in case of invalid param in strftime #4530

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
Merged
merged 2 commits into from
Feb 21, 2023
Merged

Return arg in case of invalid param in strftime #4530

merged 2 commits into from
Feb 21, 2023

Conversation

itsankitkp
Copy link
Contributor

Chrono panics in case of unsupported formats, this patch handles such cases and returns supplied format as a result.

@itsankitkp
Copy link
Contributor Author

itsankitkp commented Feb 20, 2023 •
edited by youknowone
Loading

This is re-implementation of PR: #4474
as previous PR got messed up git history

@itsankitkp itsankitkp mentioned this pull request Feb 20, 2023
Closed
@itsankitkp
Return arg in case of invalid param in strftime
fcfe8e1 
Chrono panics in case of unsupported formats, this patch handles such
cases and returns supplied format as a result.
youknowone
youknowone requested changes Feb 20, 2023
View reviewed changes
Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

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

looks good except for the comment.

you don't need to open a new pr to fix branch. please ask help next time. I can help you to fix them 😄

@itsankitkp itsankitkp force-pushed the handle-panic-strftime-new branch from 71a8095 to fcfe8e1 Compare February 20, 2023 19:28
youknowone
youknowone approved these changes Feb 21, 2023
View reviewed changes
Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

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

Thank you for contributing!

@youknowone youknowone merged commit 2e27587 into RustPython:main Feb 21, 2023
@youknowone
Copy link
Member

https://github.com/RustPython/RustPython/pull/4537/files#diff-d925268c057737ba320c489ca302fd5125f14840d22e50cfe226bdb5bb88ba8dR546-R553

This was referenced Feb 23, 2023
Open
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youknowone youknowone youknowone 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.
2 participants
@itsankitkp @youknowone