Skip to content

feat: Update @vscode/codicons to v0.0.33#1259

Merged
dsmmcken merged 1 commit intomainfrom
dmckenzie_icon_bump
May 4, 2023
Merged

feat: Update @vscode/codicons to v0.0.33#1259
dsmmcken merged 1 commit intomainfrom
dmckenzie_icon_bump

Conversation

@dsmmcken
Copy link
Copy Markdown
Contributor

@dsmmcken dsmmcken commented May 2, 2023

Update adds several new icons see https://github.com/microsoft/vscode-codicons/releases/tag/0.0.33 for changes.

They renamed circle-large-outline to circle-large for naming consistency. I've fixed our usage of vsCircleLargeOutline to vsCircleLarge for the renamed icon (no change in visuals).

BREAKING CHANGE: vsCircleLargeOutline icon renamed to vsCircleLarge

They renamed circle-large-outline to circle-large for consistency. Update adds several new icons.

I've fixed our usage to match for the renamed icon.
@dsmmcken dsmmcken self-assigned this May 2, 2023
@dsmmcken dsmmcken requested a review from mattrunyon May 2, 2023 19:53
@codecov
Copy link
Copy Markdown

codecov Bot commented May 2, 2023

Codecov Report

Merging #1259 (21db7da) into main (769d753) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1259   +/-   ##
=======================================
  Coverage   45.57%   45.57%           
=======================================
  Files         484      484           
  Lines       34077    34077           
  Branches     8532     8532           
=======================================
  Hits        15531    15531           
  Misses      18495    18495           
  Partials       51       51           
Flag Coverage Δ
unit 45.57% <ø> (ø)

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

Impacted Files Coverage Δ
packages/components/src/LoadingSpinner.tsx 100.00% <ø> (ø)

Copy link
Copy Markdown
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

We should still add Breaking Change to this PR, as we're no longer exporting vsCircleLargeOutline from @deephaven/icons

@dsmmcken
Copy link
Copy Markdown
Contributor Author

dsmmcken commented May 3, 2023

True. I just checked and enterprise does use vsCircleLargeOutline in QueryStatusIcon.

@dsmmcken dsmmcken requested a review from mofojed May 3, 2023 15:37
@mofojed
Copy link
Copy Markdown
Member

mofojed commented May 3, 2023

Another option would be to re-export it with the old name to reduce breakage? But may be more hassle than it's worth.

@dsmmcken
Copy link
Copy Markdown
Contributor Author

dsmmcken commented May 3, 2023

better to let it break and update the other side I think, the name change makes sense.

@dsmmcken dsmmcken merged commit 1b29af1 into main May 4, 2023
@dsmmcken dsmmcken deleted the dmckenzie_icon_bump branch May 4, 2023 14:59
@github-actions github-actions Bot locked and limited conversation to collaborators May 4, 2023
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.

2 participants