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(topic): Improve topic policy json diff check #1787

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

x4b1
Copy link
Contributor

@x4b1 x4b1 commented Jun 21, 2023

Description of your changes

In the observed policy on the topic, the comparison of JSON strings was changed to comparing actual JSON objects to prevent the update loop. This way it recognizes that the two objects are the same.

I have:

How has this code been tested

I have added a unit test to check this case.

@x4b1
Copy link
Contributor Author

x4b1 commented Jun 23, 2023

Hi! @MisterMX,

Can you take a look at this?

Thanks in advance.

Copy link
Collaborator

@MisterMX MisterMX left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a lot @x4b1. Can you fix the linter issues? Then this is ready merge.

@MisterMX
Copy link
Collaborator

@x4b1 can you sqash your changes into a single commit and get rid of the merge commit? This would help a lot to keep the history short and concise.

@x4b1 x4b1 force-pushed the master branch 3 times, most recently from 9ecd126 to 759fd6d Compare June 26, 2023 11:07
@x4b1
Copy link
Contributor Author

x4b1 commented Jun 26, 2023

@MisterMX done!

@MisterMX
Copy link
Collaborator

Thanks, @x4b1!

@MisterMX MisterMX merged commit 9205a21 into crossplane-contrib:master Jun 26, 2023
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