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

[4483] Safely lowercase strings for case insensitive comparison #4499

Merged
merged 7 commits into from Dec 7, 2021

Conversation

michaelrogers
Copy link
Contributor

@michaelrogers michaelrogers commented Nov 22, 2021

Closes #4483

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Is this change backwards compatible? For example, developers won't need to change how they are calling the API or how they've extended core plugins such as Tables or Plots.

Author Checklist

  • Changes address original issue?
  • Unit tests included and/or updated with changes?
  • Command line build passes?
  • Has this been smoke tested?
  • Testing instructions included in associated issue?
    Screen Shot 2021-11-22 at 3 11 06 PM

@michaelrogers michaelrogers changed the title Safely lowercase strings for case insensitive comparison [4483] Safely lowercase strings for case insensitive comparison Nov 22, 2021
@codecov
Copy link

codecov bot commented Dec 6, 2021

Codecov Report

Merging #4499 (eada009) into master (159563c) will increase coverage by 0.47%.
The diff coverage is 0.00%.

❗ Current head eada009 differs from pull request most recent head 87dd8e4. Consider uploading reports for the commit 87dd8e4 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4499      +/-   ##
==========================================
+ Coverage   69.36%   69.83%   +0.47%     
==========================================
  Files         994     1019      +25     
  Lines       35806    36813    +1007     
  Branches     1629     1589      -40     
==========================================
+ Hits        24838    25710     +872     
- Misses      10564    10697     +133     
- Partials      404      406       +2     
Impacted Files Coverage Δ
src/ui/layout/mct-tree.vue 12.88% <0.00%> (ø)
src/plugins/move/MoveAction.js 30.64% <0.00%> (-23.09%) ⬇️
src/plugins/myItems/pluginSpec.js 96.77% <0.00%> (-3.23%) ⬇️
src/ui/layout/CreateButton.vue 25.00% <0.00%> (-3.00%) ⬇️
src/ui/router/ApplicationRouter.js 85.26% <0.00%> (-1.06%) ⬇️
src/MCT.js 95.96% <0.00%> (-0.13%) ⬇️
src/plugins/duplicate/DuplicateTask.js 97.64% <0.00%> (-0.11%) ⬇️
src/api/api.js 100.00% <0.00%> (ø)
src/plugins/plugins.js 89.70% <0.00%> (ø)
src/plugins/plan/plugin.js 100.00% <0.00%> (ø)
... and 105 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 159563c...87dd8e4. Read the comment docs.

@michaelrogers
Copy link
Contributor Author

@davetsay - The external lowercasing function + string casting was removed in favor of lowercasing inline. Let me know if any further changes are needed before merging.

Copy link
Contributor

@davetsay davetsay left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewer Checklist

Changes appear to address issue? Y
Appropriate unit tests included? N/A
Code style and in-line documentation are appropriate? Y
Commit messages meet standards? Y
Has associated issue been labelled unverified? Y

@davetsay davetsay enabled auto-merge (squash) December 7, 2021 21:59
@davetsay davetsay merged commit 6bdea20 into master Dec 7, 2021
@davetsay davetsay deleted the mct4483 branch December 7, 2021 22:06
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.

Alphabetical sorting should ignore case
3 participants