Skip to content

Mailbox stubs#3427

Merged
JelleZijlstra merged 9 commits intopython:masterfrom
CraftSpider:mailbox-stubs
Nov 23, 2019
Merged

Mailbox stubs#3427
JelleZijlstra merged 9 commits intopython:masterfrom
CraftSpider:mailbox-stubs

Conversation

@CraftSpider
Copy link
Copy Markdown
Contributor

Add stubs for the mailbox module. My first contribution using new Protocols and Generics

@CraftSpider
Copy link
Copy Markdown
Contributor Author

Pytype failure is unusual, opened a bug google/pytype#444

Comment thread stdlib/2and3/mailbox.pyi Outdated
Copy link
Copy Markdown
Member

@JelleZijlstra JelleZijlstra 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 the PR! I left some suggestions.

Comment thread stdlib/2and3/mailbox.pyi Outdated
Comment thread stdlib/2and3/mailbox.pyi Outdated
Comment thread stdlib/2and3/mailbox.pyi Outdated
Comment thread stdlib/2and3/mailbox.pyi Outdated
Comment thread stdlib/2and3/mailbox.pyi Outdated
Comment thread stdlib/2and3/mailbox.pyi Outdated
Comment thread stdlib/2and3/mailbox.pyi Outdated
Comment thread stdlib/2and3/mailbox.pyi Outdated
Comment thread stdlib/2and3/mailbox.pyi Outdated
@CraftSpider
Copy link
Copy Markdown
Contributor Author

Any updates on this? I believe all review concerns were addressed, though I understand if you're just busy/taking a break.

Copy link
Copy Markdown
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Sorry for the delay! I noticed one small issue, which I just committed a fix for. I'll merge after the tests pass.

Comment thread stdlib/2and3/mailbox.pyi Outdated
class MaildirMessage(Message):

def get_subdir(self) -> str: ...
def set_subdir(self, subdir: Union[Literal["new", "cur"]]) -> None: ...
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.

No need for the Union[]

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.

2 participants