Skip to content

Conversation

@josvazg
Copy link
Collaborator

@josvazg josvazg commented Jul 10, 2024

All Submissions:

  • Have you signed our CLA?

Signed-off-by: jose.vazquez <jose.vazquez@mongodb.com>
}

*strPtr = str[len(str)-charCount:]
func areIntegrationsEqual(_, _ *aliasThirdPartyIntegration) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably a matter of taste but I would extract this into an interface and provide an "AlwaysApplyComparer" or something along these lines. Just to get this stop-the-bleeding patch code out of the sight.

Feel free to ignore this suggestion if you think otherwise.

Copy link
Collaborator Author

@josvazg josvazg Jul 10, 2024

Choose a reason for hiding this comment

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

Not sure is worth it. We are going to actually write quite some new code once we need to actually compare. We just do not know what that code is now, so having interfaces seems premature. For instance, if we go for implicit evaluation these interfaces break, because we need input from 3 sources, Atlas, Spec and annotations.

@github-actions
Copy link
Contributor

Copy link
Collaborator

@s-urbaniak s-urbaniak left a comment

Choose a reason for hiding this comment

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

lgtm but I'd like to leave a final review to @helderjs and/or @igor-karpukhin as well.

@s-urbaniak s-urbaniak merged commit 2711915 into main Jul 12, 2024
@s-urbaniak s-urbaniak deleted the CLOUDP-260438/fix-integration-comparisons branch July 12, 2024 10:43
josvazg added a commit that referenced this pull request Jul 12, 2024
s-urbaniak pushed a commit that referenced this pull request Jul 12, 2024
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.

6 participants