-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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 to properly check for 'undefined' #684
Conversation
@Andre-Nunes these look good, thanks. The only thing that sticks out to me is that the formatting for You are welcome to keep this pull request open and continue to commit to your branch until you are ready for a larger review. |
Ok, I'll fix the formatting. I'll keep commiting untill all the files I've pinpointed are fixed. |
Ok, I'm done with Core/, I've noticed #694 so I guess I should hold off on this issue until both pull requests are sorted. Let me know if something comes up. |
Thanks a lot @Andre-Nunes! This cleanup is long overdue and it's nice to see you taking charge of it. |
Fix to properly check for 'undefined'
@Andre-Nunes I don't know if you noticed my mailing list email, but it turns our there were still some problems with your branch that I didn't pick up. I fixed them (since I was the one that took them into master before they were ready) but I wanted to provide you some additional feedback anyway.
Anyway, thanks again for the changes and I'm sorry for the confusion! I look forward to more pull requests from you in the future. |
Thanks for being so cool about it. I will keep that in mind for the future! |
Thanks @Andre-Nunes. These are good changes, and many are complimentary to #694. |
Still working on issue #89, just to make sure I'm on the right track as I've pinpointed every file in Source/Core that needs to be fixed.