Skip to content

Adding DataDescriptionMatcher operation 'Not'#8565

Merged
matthiasrichter merged 1 commit into
AliceO2Group:devfrom
matthiasrichter:dev-matcherop
Apr 25, 2022
Merged

Adding DataDescriptionMatcher operation 'Not'#8565
matthiasrichter merged 1 commit into
AliceO2Group:devfrom
matthiasrichter:dev-matcherop

Conversation

@matthiasrichter

Copy link
Copy Markdown
Collaborator

Need to check if using this negator operation can have implications
on the handling of concrete data types. No impact to existing code
however.

@matthiasrichter matthiasrichter requested a review from a team as a code owner April 12, 2022 12:31
@matthiasrichter

Copy link
Copy Markdown
Collaborator Author

This will disentangle #8561 from the mapping of DataMatcher to ConcreteDataMatchers as we do not need to use the xor operation.

On the other hand, introducing the 'Not'-operator might have a similar effect on the handling of concrete data descriptions.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@ktf shouldn't we return leftValue here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

think the correct logic is to return leftValue

Need to check if using this negator operation can have implications
on the handling of concrete data types. No impact to existing code
however.

if (mOp == Op::Just) {
return true;
return leftValue;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think when having two matchers with just left condition, the two should should be tested for equality and thus we need to return leftValue.

Actually, leftResult and rightresult would be better naming, shall we apply this?

@matthiasrichter

Copy link
Copy Markdown
Collaborator Author

moving on with this and merging, the CI error is unrelated

@matthiasrichter matthiasrichter merged commit 9bc95c0 into AliceO2Group:dev Apr 25, 2022
@matthiasrichter matthiasrichter deleted the dev-matcherop branch April 27, 2022 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant