Skip to content
This repository was archived by the owner on Apr 11, 2024. It is now read-only.

Change Session loading to return undefined when there is no Session#64

Merged
thecodepixi merged 1 commit intomainfrom
change-load-session
Jan 5, 2021
Merged

Change Session loading to return undefined when there is no Session#64
thecodepixi merged 1 commit intomainfrom
change-load-session

Conversation

@thecodepixi
Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

We were previously returning null when no Session is found. We should instead return undefined

WHAT is this pull request doing?

Updates all Session loading implementation return types to Session | undefined

Type of change

  • Minor: New feature (non-breaking change which adds functionality)

@thecodepixi thecodepixi requested a review from a team as a code owner January 5, 2021 16:33
@thecodepixi thecodepixi changed the title Change Session loading to return undefined when there is no Session [WIP] Change Session loading to return undefined when there is no Session Jan 5, 2021
@thecodepixi thecodepixi changed the title [WIP] Change Session loading to return undefined when there is no Session Change Session loading to return undefined when there is no Session Jan 5, 2021
@tanema
Copy link
Copy Markdown
Contributor

tanema commented Jan 5, 2021

We should instead return undefined

Why?

Copy link
Copy Markdown
Contributor

@tanema tanema left a comment

Choose a reason for hiding this comment

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

I can see a lot of changes that are more linting than the specific change for this PR. Ideally these would be separate PRs (or at least only changing the code you touch) But it LGTM anyway 👍

Copy link
Copy Markdown
Contributor

@paulomarg paulomarg left a comment

Choose a reason for hiding this comment

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

The changes LGTM!

@thecodepixi
Copy link
Copy Markdown
Contributor Author

I can see a lot of changes that are more linting than the specific change for this PR. Ideally these would be separate PRs (or at least only changing the code you touch) But it LGTM anyway 👍

Yeah, this seems to be happening to me a lot lately. I think we/I might have some linting inconsistencies so when I pull things down it gets formatted on my end. Will fix that ASAP.

@thecodepixi
Copy link
Copy Markdown
Contributor Author

We should instead return undefined

Why?

My personal thinking here is that a session is almost always assigned to a variable, and if a variable has no value it is therefore undefined, and also that functions generally return undefined if there is no return value. My understanding of null is "intentional nothingness", and we expect that the session loading functions should return something, if they don't, the session is just undefined, rather than the session being null/nothing.

@thecodepixi thecodepixi merged commit 52b9326 into main Jan 5, 2021
@thecodepixi thecodepixi deleted the change-load-session branch January 5, 2021 20:24
@paulomarg paulomarg temporarily deployed to production January 13, 2021 19:45 Inactive
@paulomarg paulomarg temporarily deployed to production January 27, 2021 19:19 Inactive
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.

3 participants