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(manager): use typeof function instead of directly comparison #1481

Merged
merged 4 commits into from
Feb 2, 2024

Conversation

murka
Copy link
Contributor

@murka murka commented Jan 24, 2024

@murka murka requested a review from a team as a code owner January 24, 2024 17:17
fortuna
fortuna previously approved these changes Jan 24, 2024
Copy link
Collaborator

@fortuna fortuna left a comment

Choose a reason for hiding this comment

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

I don't understand, how is that an issue? It's working. The links you sent are for Java?

@murka
Copy link
Contributor Author

murka commented Jan 24, 2024

It is just example on Java.

This is recommendation: https://codeql.github.com/codeql-query-help/javascript/js-comparison-between-incompatible-types/

Copy link
Contributor

@daniellacosse daniellacosse left a comment

Choose a reason for hiding this comment

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

It needs to be the string "undefined" not the undefined object

@fortuna fortuna dismissed their stale review January 24, 2024 18:33

Re-review

@fortuna fortuna changed the title fix(server_manager): use typeof function instead of directly comparison fix(manager): use typeof function instead of directly comparison Jan 24, 2024
Copy link
Contributor Author

@murka murka left a comment

Choose a reason for hiding this comment

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

All the workflows are passed! The PR is ready for merge, imho :)

https://github.com/murka/outline-server/actions/runs/7652358841

@fortuna
Copy link
Collaborator

fortuna commented Jan 29, 2024

@daniellacosse another look?

@daniellacosse
Copy link
Contributor

daniellacosse commented Jan 31, 2024 via email

…gs.ts

Co-authored-by: Vinicius Fortuna <fortuna@users.noreply.github.com>
@murka murka requested a review from daniellacosse February 1, 2024 07:02
@fortuna
Copy link
Collaborator

fortuna commented Feb 1, 2024

@murka it seems like the test failed. Can you take a look?
npm run action server_manager/test

@murka
Copy link
Contributor Author

murka commented Feb 1, 2024

@fortuna, I fix that. All jobs is done: https://github.com/murka/outline-server/actions/runs/7743696038/

@fortuna fortuna merged commit 882573e into Jigsaw-Code:master Feb 2, 2024
14 checks passed
@murka murka deleted the fix-server_manager branch February 6, 2024 03: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.

3 participants