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 is_conditional_active(...) #3655

Merged
merged 4 commits into from
Jun 16, 2022

Conversation

DmitryAk
Copy link
Contributor

@DmitryAk DmitryAk commented Jun 15, 2022

Issue

#3656
Found during static code analyzer running
The same condition is checked twice:

if (type == kYMD && (b_month && e_month) && (!b_day_dow && !e_day_dow && !b_week && !b_week) &&

It seems like in the second case !e_week should be checked.

What is the best way to cover this with test?

Tasklist

@@ -365,7 +365,7 @@ bool is_conditional_active(const bool type,

bool edge_case = false; // Jan 04 to Jan 01
// month only
if (type == kYMD && (b_month && e_month) && (!b_day_dow && !e_day_dow && !b_week && !b_week) &&
if (type == kYMD && (b_month && e_month) && (!b_day_dow && !e_day_dow && !b_week && !e_week) &&
Copy link
Member

Choose a reason for hiding this comment

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

such an easy copy paste fail!

CHANGELOG.md Outdated Show resolved Hide resolved
@kevinkreiser kevinkreiser merged commit b426ffb into valhalla:master Jun 16, 2022
@DmitryAk DmitryAk deleted the da-fix-is_conditional_active branch August 15, 2022 06: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

2 participants