Skip to content

Conversation

rruuaanng
Copy link
Contributor

@rruuaanng rruuaanng commented Sep 6, 2024

After getting the email encoding format, added strip() to remove unexpected spaces

@bedevere-app
Copy link

bedevere-app bot commented Sep 6, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app

This comment was marked as duplicate.

@rruuaanng rruuaanng changed the title gh-123742: After getting the email encoding format, added strip() to remove unexpected spaces gh-123742: Fixed get_payload not being able to parse headers with spaces Oct 15, 2024
@rruuaanng

This comment was marked as outdated.

@erlend-aasland
Copy link
Contributor

@rruuaanng, I will point again to the devguide and suggest that you read it again. The Getting Started section states that we avoid force-pushing; it messes up the review process and does not play well with CI feedback.

@rruuaanng

This comment was marked as off-topic.

@erlend-aasland

This comment was marked as off-topic.

Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

Previous review remarks were not addressed; please address them.

@bedevere-app
Copy link

bedevere-app bot commented Nov 5, 2024

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

Can you please add a test for this?

@rruuaanng
Copy link
Contributor Author

It has been a long time since this PR was opened. If there are no problems, I hope we can approve it :)

@rruuaanng
Copy link
Contributor Author

I discussed this with Bénédikt in #123742 and got some feedback. I think we can follow the conclusion of practice, which is to remove trailing spaces, otherwise it will cause parsing errors, even though the RFC is vague about this, but I think making the module work properly is our ultimate pursuit.

@rruuaanng
Copy link
Contributor Author

Thank you, Erlend, for your repeated review of this. But now, I will close this PR, It has been solved.

@rruuaanng rruuaanng closed this Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants