deduplicate the 'isSomeFunction' equivalent#7749
Conversation
|
Thanks for your pull request, @UplinkCoder! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
|
FYI: This is a resubmission of #6835 because the Phantom Zone thing didn't work out well. We couldn't re-open the PR after @UplinkCoder made changes. |
| a delegate, function, function-pointer | ||
| or a variable of the former. Otherwise, `null`. | ||
| */ | ||
| static TypeFunction toTypeFunction(RootObject o, FuncDeclaration* fdp = null) |
There was a problem hiding this comment.
Continuing our discussion from Slack: I completely overlooked that this was a nested function. In that case, yes, please disregard my previous comments (Sorry!). But none of the other nested functions here are static. I suggest removing static.
Otherwise, what I was implying with my previous comments is that this is a utility function, and might be better as a private free function of the module. I'm not going to require it though. your call.
UPDATE: In summary, either remove the static (or justify it), or make the function a private free function in the module. I leave it up to you to decide.
There was a problem hiding this comment.
UPDATE 2: OK, I just realized the outer function context is not required, so static seems to make sense. I'm good, now. Sorry to beat this horse so much.
JinShil
left a comment
JinShil
left a comment
There was a problem hiding this comment.
I'm going to leave a short window of opportunity for others to add their review before I merge.
|
My review is here #7754 :-) |
4 participants
as per title