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 weatheralerts: "Special Weather" Alert Type Not Found #217

Merged
merged 4 commits into from
Oct 12, 2023

Conversation

shaulbd
Copy link
Contributor

@shaulbd shaulbd commented Oct 6, 2023

Fixes: #206

In the original code, the !alertType condition was used to check for a missing alert type. However, it had the unintended effect of returning true for alert types with a value of 0. This was problematic because 0 was used for "Special Weather" and "Special Marine".

The modified code uses alertType === undefined to explicitly check for a missing alert type while preserving the expected behavior for other valid numbered values.

@MrBartusek
Copy link
Owner

Hey, thanks for this PR! There is something wrong with the new line characters in this PR. I've added .gitattributes file to the master, please rebase this PR and it should fix itself.

@MrBartusek MrBartusek added bug Something isn't working integration: weatheralerts labels Oct 7, 2023
@MrBartusek
Copy link
Owner

image

@MrBartusek
Copy link
Owner

I converted the file to LF and git fixed it automatically apparently

Copy link
Owner

@MrBartusek MrBartusek left a comment

Choose a reason for hiding this comment

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

Hey thanks for this PR! It indeed seems to fix this problem! Can you also fix alert level check likewise?

if(!alertLevel) {
throw Error(`Unknown weatheralerts alert level: ${fullAlertName}`);
}

@MrBartusek MrBartusek self-requested a review October 12, 2023 06:53
Copy link
Owner

@MrBartusek MrBartusek left a comment

Choose a reason for hiding this comment

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

Thank you!

@MrBartusek MrBartusek changed the title Fix error: "Special Weather" Alert Type Not Found Fix weatheralerts: "Special Weather" Alert Type Not Found Oct 12, 2023
@MrBartusek MrBartusek merged commit fcd2b78 into MrBartusek:master Oct 12, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working integration: weatheralerts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Special Weather" Alert Type Not Found
2 participants