Fixed some warnings by JaviSoto · Pull Request #1 · FLEXTool/FLEX · GitHub
Skip to content

Fixed some warnings#1

Merged
ryanolsonk merged 3 commits into
FLEXTool:masterfrom
JaviSoto:master
Jul 29, 2014
Merged

Fixed some warnings#1
ryanolsonk merged 3 commits into
FLEXTool:masterfrom
JaviSoto:master

Conversation

@JaviSoto

Copy link
Copy Markdown
Contributor

No description provided.

JaviSoto added 2 commits July 25, 2014 10:40
Not a huge deal in this case, but it's always a good idea to get a strong reference to the object we only have a weak reference to so that it can't go away half-way through the execution of a block (which in some cases can cause bugs that are super hard to track down)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why don't you use NSUInteger instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because class_copyMethodList returns an unsigned int.

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.

👍

@JaviSoto

Copy link
Copy Markdown
Contributor Author

The code change looks good . The indentation on the first line looks off.

Just fixed that :D Sorry about that (our project uses 2 spaces for legacy reasons...)

@ryanolsonk

Copy link
Copy Markdown
Member

I really appreciate the attention to consistency with the existing code. We'll get this merged as soon as the CLA is finalized.

@JaviSoto

Copy link
Copy Markdown
Contributor Author

I tried :) Consistency is important!

@ryanolsonk

Copy link
Copy Markdown
Member

@JaviSoto

Copy link
Copy Markdown
Contributor Author

Done :)

@ryanolsonk

Copy link
Copy Markdown
Member

ryanolsonk added a commit that referenced this pull request Jul 29, 2014
@ryanolsonk ryanolsonk merged commit 4e0313a into FLEXTool:master Jul 29, 2014
@studentdeng studentdeng mentioned this pull request Aug 14, 2014
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.

3 participants