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.

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

Added underscore as dependency to section_changer.js #35697

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

orangejenny
Copy link
Contributor

Technical Summary

Followup to #35693, which I still swear I did with a script.

For good measure, I viewed the six files edited in that PR, searched for $, ko, and _., and verified there weren't any other misses.

Safety Assurance

Safety story

Adds a dependency on a library that's always available. Quite safe.

Automated test coverage

no

QA Plan

no

Rollback instructions

  • This PR can be reverted after deploy with no further considerations

Labels & Review

  • Risk label is set correctly
  • The set of people pinged as reviewers is appropriate for the level of risk of the change

@orangejenny orangejenny added the product/invisible Change has no end-user visible impact label Jan 28, 2025
], function (
$
$,
_,
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the comma here cause issues? Aka is there a reason we didn't have it there before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ES5 doesn't allow trailing commas for parameters lists in function calls, that was added in ES6. It's not a problem in browsers, which have moved on from ES5, but for code using requirejs, it'll break deploy (because uglify will fail to parse the file). HQ has been using ES5 in linting rules forever for this reason.

#35651 updated HQ to lint using ES6, now that almost all of our code is off of requirejs. The new lint config expects trailing commas in function calls.

So it should have been there before, I just haven't gotten in the habit yet...that might also be more evidence that I did this file by hand before scripting, since spot checking the related PRs, they have trailing commas for the hqDefine callback arguments.

@orangejenny orangejenny merged commit 961b0d2 into master Jan 28, 2025
13 checks passed
@orangejenny orangejenny deleted the jls/section-changer-js branch January 28, 2025 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product/invisible Change has no end-user visible impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants