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

chore(rollouts): account for segments without constraints #1919

Merged
merged 2 commits into from
Jul 28, 2023

Conversation

yquansah
Copy link
Contributor

@yquansah yquansah commented Jul 28, 2023

Add logic to account for segments without constraints. For rules, segments without constraints result in a match during evaluation. This PR does the following:

  • Uses a LEFT JOIN instead of a JOIN in the query to get segments that do not have constraints out of the db
  • Writes unit/integration tests to validate that the behavior is working

@yquansah yquansah requested a review from a team as a code owner July 28, 2023 15:48
Value: "bar",
})

require.NoError(t, err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the CreateConstraint here to ensure that the query still picks up the necessary information.

@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Merging #1919 (9dfe7c2) into main (8bb0e95) will decrease coverage by 0.10%.
Report is 14 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1919      +/-   ##
==========================================
- Coverage   71.35%   71.26%   -0.10%     
==========================================
  Files          66       66              
  Lines        6455     6455              
==========================================
- Hits         4606     4600       -6     
- Misses       1592     1596       +4     
- Partials      257      259       +2     

see 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@markphelps markphelps left a comment

Choose a reason for hiding this comment

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

lgtm!

@yquansah yquansah merged commit 31ba28e into main Jul 28, 2023
@yquansah yquansah deleted the optional-constraint branch July 28, 2023 17:00
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