Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: do not test for inferred names in getFnName unit test #10990

Merged
merged 3 commits into from Feb 27, 2018
Merged

fix: do not test for inferred names in getFnName unit test #10990

merged 3 commits into from Feb 27, 2018

Conversation

DanielRuf
Copy link
Contributor

@DanielRuf DanielRuf commented Feb 26, 2018 •
edited
Loading

@DanielRuf DanielRuf mentioned this pull request Feb 26, 2018
Closed
ncoden
ncoden reviewed Feb 26, 2018
View reviewed changes
Copy link
Contributor

@ncoden ncoden left a comment

Choose a reason for hiding this comment

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

👍

test/javascript/util/core.js
it('should handle a function expression', function() {
var B = function(){};
var name = Foundation.getFnName(B);
it('should handle an anonymous function expression', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment about the "inconsistent behavior" ?
Like:

// Inferred names (i.e. `var name = function() {}`) are not tested as they are widely supported
// See: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/name#Inferred_function_names

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@ncoden ncoden merged commit f29e2d2 into foundation:develop Feb 27, 2018
@DanielRuf DanielRuf deleted the fix/get-fn-name-unit-test branch February 28, 2018 04:49
@ncoden ncoden added this to the 6.5.0 milestone Apr 23, 2018
ncoden pushed a commit to ncoden/foundation-sites that referenced this pull request Jun 16, 2018
@DanielRuf @ncoden
Use pull request foundation#10990 from DanielRuf/fix/get-fn-name-unit…
c6182a2 
…-test for v6.5.0

406cb1b fix: do not test for inferred names in getFnName unit test
5071568 fix small typo
afec54b add info about inferred names of anonymous functions

Signed-off-by: Nicolas Coden <nicolas@ncoden.fr>
This was referenced Jun 16, 2018
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ncoden ncoden ncoden left review comments

Assignees
No one assigned
Labels
javascript utilities javascript 🚨tests
Projects
None yet
Milestone
6.5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@DanielRuf @ncoden