Cleanups #306

Open
wants to merge 6 commits into
from

Conversation

Projects
None yet
2 participants
@sorced-jim

I personally think factoring out these bits of code makes things easier to read. I believe it will make it easier to integrate SPDY as well when I get to that.

+ }
+}
+
+// url.Host and url.scheme must be set when this method is called.

This comment has been minimized.

Show comment Hide comment
@erikchen

erikchen Mar 7, 2012

Collaborator

The old code looked like it tried to handle the case where url.Host and url.scheme were not set.
If you are assuming they are set, you should assert that they are not nil.
Where in your code are you actually enforcing that Host and scheme exist?

@erikchen

erikchen Mar 7, 2012

Collaborator

The old code looked like it tried to handle the case where url.Host and url.scheme were not set.
If you are assuming they are set, you should assert that they are not nil.
Where in your code are you actually enforcing that Host and scheme exist?

This comment has been minimized.

Show comment Hide comment
@sorced-jim

sorced-jim Mar 8, 2012

I ensure it before calling this method. I've added the asserts you were looking for and have fix the reference counts so Xcode analyzer now likes me.

@sorced-jim

sorced-jim Mar 8, 2012

I ensure it before calling this method. I've added the asserts you were looking for and have fix the reference counts so Xcode analyzer now likes me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment