-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix all project coding warnings in examples
folder
#1582
Conversation
ec986b8
to
6dda57c
Compare
🚫 CI failed with log |
Could you trigger the CI again? Because it seems the CI failing is not related to the changed code, according to the error message ^_^
|
@@ -58,7 +58,8 @@ - (ASCellNode *)tableNode:(ASTableNode *)tableNode nodeForRowAtIndexPath:(NSInde | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should migrate to the -nodeBlock
data source method as well. Not a blocker for this PR though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find there're a few other example projects which also need to migrate to the -nodeBlock
data source method, will create a new PR for fixing this issue altogether :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @nguyenhuy , the PR is ready at #1596
@@ -58,7 +58,8 @@ - (ASCellNode *)tableNode:(ASTableNode *)tableNode nodeForRowAtIndexPath:(NSInde | |||
{ | |||
RandomCoreGraphicsNode *elementNode = [[RandomCoreGraphicsNode alloc] init]; | |||
elementNode.style.preferredSize = _elementSize; | |||
elementNode.indexPath = [NSIndexPath indexPathForRow:indexPath.row inSection:_pageNumber]; | |||
NSString *content = [[NSIndexPath indexPathForRow:indexPath.row inSection:_pageNumber] description]; | |||
[elementNode setContent:content]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the content is set only once, maybe pass it in as a param of the initializer and remove -setContent:
altogether?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 00378ef7.
You got a green light from CI btw. |
Fix all project coding warnings in
examples
folder