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

DPL-1047: Cleanup - upgrade code linting to ES9 standard #1869

Merged
merged 27 commits into from
Aug 30, 2024

Conversation

StephenHulme
Copy link
Contributor

Closes #1555

Changes proposed in this pull request

  • Consolidates ESLint rules
  • Addresses linting warnings
  • Removes IIFEs (Immediately Invoked Function Expression)
  • Loads module code where needed on a per-page basis

Instructions for Reviewers

[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to main]
    - Check story numbers included
    - Check for debug code
    - Check version

@StephenHulme StephenHulme changed the title DPL-1047: upgrade code linting to ES9 standard DPL-1047: Cleanup - upgrade code linting to ES9 standard Aug 22, 2024
@StephenHulme StephenHulme self-assigned this Aug 22, 2024
Copy link

codecov bot commented Aug 23, 2024

Codecov Report

Attention: Patch coverage is 0.46154% with 1294 lines in your changes missing coverage. Please review.

Project coverage is 74.95%. Comparing base (6aec2b9) to head (8c455ec).
Report is 28 commits behind head on develop.

Files with missing lines Patch % Lines
.../frontend/entrypoints/pages/multi_plate_pooling.js 0.00% 339 Missing ⚠️
app/frontend/entrypoints/pages/tagged-plate.js 0.00% 217 Missing ⚠️
app/frontend/entrypoints/pages/pooled_tubes.js 0.00% 186 Missing ⚠️
app/frontend/entrypoints/pages/bed_verification.js 0.00% 169 Missing ⚠️
...p/frontend/entrypoints/pages/multi_tube_pooling.js 0.00% 161 Missing ⚠️
app/frontend/javascript/legacy_scripts_a.js 0.00% 79 Missing ⚠️
app/frontend/javascript/session_scripts.js 0.00% 54 Missing ⚠️
app/frontend/javascript/state_machine.js 0.00% 25 Missing ⚠️
...p/frontend/javascript/lib/global_message_system.js 0.00% 23 Missing ⚠️
app/frontend/javascript/state_change_reasons.js 0.00% 13 Missing ⚠️
... and 9 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1869      +/-   ##
===========================================
+ Coverage    74.54%   74.95%   +0.41%     
===========================================
  Files          421      420       -1     
  Lines        14379    14300      -79     
===========================================
  Hits         10719    10719              
+ Misses        3660     3581      -79     
Flag Coverage Δ
javascript 60.27% <0.46%> (+0.62%) ⬆️
pull_request 74.95% <0.46%> (?)
push 74.95% <0.46%> (+0.41%) ⬆️
ruby 91.19% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@sdjmchattie sdjmchattie left a comment

Choose a reason for hiding this comment

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

I'm not super familiar with the changes taking place here, but they look fine. Git isn't doing a very nice job of aligning the removed/added lines in the diff where very little has changed. Main things I observed were the change of jquery imports to use $ as the imported name and changing some var usage to let. Indent changes I wasn't sure why they happened, but it looks fine and obviously it works.

@StephenHulme
Copy link
Contributor Author

Indent changes I wasn't sure why they happened, but it looks fine and obviously it works.

The individual scripts were previously wrapped in local functions. Removing these anonymous wrappers is the cause of the spacing changes. Since ES6, modules are scoped by default and the wrappers are no longer necessary.

Copy link

codeclimate bot commented Aug 30, 2024

Code Climate has analyzed commit 8c455ec and detected 11 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 4
Duplication 7

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 91.1% (0.0% change).

View more on Code Climate.

@StephenHulme StephenHulme merged commit 0ee4b40 into develop Aug 30, 2024
20 of 21 checks passed
@StephenHulme StephenHulme deleted the dpl-1047-lint-to-es9-standard branch August 30, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants