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

Move key types to central location #7531

Merged
merged 6 commits into from
Jan 17, 2024

Conversation

MGibson1
Copy link
Member

Type of change

- [ ] Bug fix
- [ ] New feature development
- [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

Moves key types to a central type file. I plan on expanding these opaque types to include asymmetric keys, which make sense in a single key type file rather than distributed between multiple.

All changes are either moving these opaque types or import updates.

Before you submit

  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team
  • Ensure that all UI additions follow WCAG AA requirements

@MGibson1 MGibson1 requested review from a team as code owners January 11, 2024 22:31
@github-actions github-actions bot added the needs-qa Marks a PR as requiring QA approval label Jan 11, 2024
@MGibson1 MGibson1 removed the needs-qa Marks a PR as requiring QA approval label Jan 11, 2024
Copy link

codecov bot commented Jan 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (756c02c) 63.01% compared to head (5fdca87) 63.01%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7531   +/-   ##
=======================================
  Coverage   63.01%   63.01%           
=======================================
  Files         760      760           
  Lines       21525    21525           
  Branches     4272     4272           
=======================================
  Hits        13563    13563           
  Misses       7140     7140           
  Partials      822      822           

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

@MGibson1 MGibson1 enabled auto-merge (squash) January 11, 2024 23:32
@JaredSnider-Bitwarden
Copy link
Contributor

@MGibson1 , this is a great change, but looks like there are several build failures to address still.

@MGibson1 MGibson1 requested a review from a team as a code owner January 15, 2024 15:31
@MGibson1 MGibson1 force-pushed the ps/move-key-types-to-central-location branch 2 times, most recently from 3954a2b to 812ff95 Compare January 15, 2024 16:33
@MGibson1 MGibson1 force-pushed the ps/move-key-types-to-central-location branch from 812ff95 to ce9b3ec Compare January 15, 2024 16:41
@MGibson1 MGibson1 requested a review from a team as a code owner January 15, 2024 16:41
djsmith85
djsmith85 previously approved these changes Jan 15, 2024
Copy link
Contributor

@JaredSnider-Bitwarden JaredSnider-Bitwarden left a comment

Choose a reason for hiding this comment

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

LGTM!

Hinton
Hinton previously approved these changes Jan 16, 2024
Copy link
Member

Choose a reason for hiding this comment

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

Why a .d.ts file? TypeScript maintainers aren't a fan of using them for this. microsoft/TypeScript#52593 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

didn't know that, I'd always thought they were indicators of types only

Jingo88
Jingo88 previously approved these changes Jan 16, 2024
cagonzalezcs
cagonzalezcs previously approved these changes Jan 16, 2024
shane-melton
shane-melton previously approved these changes Jan 16, 2024
@eliykat eliykat removed the request for review from addisonbeck January 16, 2024 21:42
@MGibson1 MGibson1 merged commit 160a636 into main Jan 17, 2024
63 checks passed
@MGibson1 MGibson1 deleted the ps/move-key-types-to-central-location branch January 17, 2024 12:27
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.

None yet

10 participants