Skip to content

Comments

Simplify '.content' by removing CONTENT_MISSING#1055

Merged
Kriechi merged 5 commits intomitmproxy:masterfrom
MatthewShao:issue#963
Mar 27, 2016
Merged

Simplify '.content' by removing CONTENT_MISSING#1055
Kriechi merged 5 commits intomitmproxy:masterfrom
MatthewShao:issue#963

Conversation

@MatthewShao
Copy link
Contributor

@MatthewShao MatthewShao commented Mar 26, 2016

fixes #963
As @mhils said, I did this in two step:

  • setting None to CONTENT_MISSING (to see if anything go wrong)
  • replacing CONTENT_MISSING with None

The relative comments and document have been updated too, hope I didn't miss anything.

content: Content of the request, None, or CONTENT_MISSING if there
is content associated, but not present. CONTENT_MISSING evaluates
to False to make checking for the presence of content natural.
content: Content of the request, the value is None if there is content
Copy link
Member

Choose a reason for hiding this comment

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

...Content of the response...

Copy link
Member

Choose a reason for hiding this comment

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

@Kriechi: ?
This seems right here.

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes - it's ok.
For some reason the Github diff view indicated a different class...
screen shot 2016-03-26 at 18 07 15

if message["content"]:
message["contentLength"] = len(message["content"])
elif message["content"] == CONTENT_MISSING:
elif message["content"] is None:
Copy link
Member

Choose a reason for hiding this comment

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

This if elif else can be simplified to if else by combining the cases where content is not None.

@mhils
Copy link
Member

mhils commented Mar 26, 2016

Other than my minor comments above, LGTM! 😃

@Kriechi Kriechi merged commit ddea343 into mitmproxy:master Mar 27, 2016
@Kriechi
Copy link
Member

Kriechi commented Mar 27, 2016

Thanks! 🎉

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.

Simplify .content

3 participants