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

NAS-131036 / 25.04 / Remove use of chain method from 'lodash-es' #10716

Merged
merged 4 commits into from
Sep 23, 2024

Conversation

RehanY147
Copy link
Contributor

@RehanY147 RehanY147 commented Sep 20, 2024

Changes:

Fixes uses of chain method from lodash-es. We should either be calling chain via _.chain or use individual methods without chaining. Using chain from import { chain } from 'lodash-es'; is not advisable because it doesn't include any of the methods so you can't call methods on the collection returned by chain method.

Testing:

Test that breadcrumbs in places like datasets/snapshots or shares/smb etc are showing up. Test in enclosure dashboard. Check console for errors.

@RehanY147 RehanY147 requested a review from a team as a code owner September 20, 2024 00:27
@RehanY147 RehanY147 requested review from denysbutenko and removed request for a team September 20, 2024 00:27
@bugclerk
Copy link
Contributor

@bugclerk bugclerk changed the title NAS-131036: Remove use of chain method from 'lodash-es' NAS-131036 / 25.04 / Remove use of chain method from 'lodash-es' Sep 20, 2024
Copy link

codecov bot commented Sep 20, 2024

Codecov Report

Attention: Patch coverage is 84.80000% with 38 lines in your changes missing coverage. Please review.

Project coverage is 80.21%. Comparing base (8e5e6c9) to head (2e6c63e).

Files with missing lines Patch % Lines
...les/page-header/breadcrumb/breadcrumb.component.ts 41.66% 7 Missing ⚠️
...oudsync/cloudsync-form/cloudsync-form.component.ts 70.00% 3 Missing ⚠️
...what-and-when/cloudsync-what-and-when.component.ts 70.00% 3 Missing ⚠️
...les/permissions/stores/dataset-acl-editor.store.ts 25.00% 3 Missing ⚠️
.../modules/entity/entity-job/entity-job.component.ts 33.33% 2 Missing ⚠️
...-as-preset-modal/save-as-preset-modal.component.ts 60.00% 2 Missing ⚠️
...p/pages/sharing/smb/smb-form/smb-form.component.ts 33.33% 2 Missing ⚠️
...connect-modal/export-disconnect-modal.component.ts 33.33% 2 Missing ⚠️
...review/existing-configuration-preview.component.ts 66.66% 2 Missing ⚠️
src/app/services/schema/app-schema.service.ts 75.00% 2 Missing ⚠️
... and 10 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10716      +/-   ##
==========================================
+ Coverage   80.19%   80.21%   +0.01%     
==========================================
  Files        1570     1570              
  Lines       51623    51626       +3     
  Branches     5834     5834              
==========================================
+ Hits        41401    41410       +9     
+ Misses      10222    10216       -6     
Flag Coverage Δ
80.21% <84.80%> (+0.01%) ⬆️

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.

@undsoft
Copy link
Collaborator

undsoft commented Sep 23, 2024

Let's also ban via linter rules.

@RehanY147 RehanY147 enabled auto-merge (squash) September 23, 2024 10:56
@RehanY147 RehanY147 merged commit b237815 into master Sep 23, 2024
6 checks passed
@RehanY147 RehanY147 deleted the NAS-131036 branch September 23, 2024 17:57
@bugclerk
Copy link
Contributor

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Sep 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants