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

fix: Made workspace non-nullable. #6300

Merged
merged 3 commits into from
Aug 2, 2022

Conversation

gonfunko
Copy link
Contributor

@gonfunko gonfunko commented Aug 2, 2022

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm run lint

The details

Resolves

Part of #5857

Proposed Changes

The workspace property of Block/BlockSvg is non-nullable.

Behavior Before Change

The workspace property was nullable, even though it was de facto never null.

Behavior After Change

The workspace is no longer nullable and doesn't need a bunch of ! and ? when referring to it.

Test Coverage

Ensured tests still pass and manually tested.

@gonfunko gonfunko requested a review from a team as a code owner August 2, 2022 20:03
@gonfunko gonfunko requested a review from BeksOmega August 2, 2022 20:03
@BeksOmega
Copy link
Collaborator

It looks like most of the references to someBlock.workspace that were checking for nullability were fixed, but the one in connection was not. I'm not sure of the best way to find all of these... the only thing I can think of is doing searches for !.*\.workspace and \(.*\.workspace and then manually auditing them. Not sure of your thoughts, but I'd like us to do some kind of check/sweep for references.

@gonfunko
Copy link
Contributor Author

gonfunko commented Aug 2, 2022

(.*.workspace

I did exactly that, but must have missed this one. Updated accordingly!

@gonfunko gonfunko merged commit 83a3e74 into google:develop Aug 2, 2022
@gonfunko gonfunko deleted the nullable-workspace branch August 2, 2022 20:36
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