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

🎉 Compare versions tool #24421

Merged
merged 4 commits into from
Mar 27, 2023
Merged

🎉 Compare versions tool #24421

merged 4 commits into from
Mar 27, 2023

Conversation

lazebnyi
Copy link
Collaborator

What

To compare in memory records outputs need some tool

How

Created a bash script that pull the images for the two docker versions and calculated the difference of the record output

Copy link
Contributor

@evantahler evantahler left a comment

Choose a reason for hiding this comment

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

Nice!

✔️ This keeps records in memory
✔️ It compares the output

Some docs might be nice, e.g. a readme or a comment at the top

  • Looks like this needs to be run from the root dir of the connector (where /secrets is relatively nearby)
  • You probably don't want to run a whole sync, so some thought should be used when choosing a starting state? Or, maybe you do want to run a "full" sync

It also might be helpful to use a visual diff tool. Here are some.

@lazebnyi lazebnyi requested review from evantahler and sherifnada and removed request for sherifnada March 23, 2023 22:31
@evantahler evantahler requested a review from sherifnada March 24, 2023 00:51
tools/internal/compare_versions.sh Outdated Show resolved Hide resolved
tools/internal/compare_versions.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

should we also be comparing STATE messages? it's an important part of the behavior

tools/internal/compare_versions.sh Outdated Show resolved Hide resolved
@lazebnyi lazebnyi requested a review from sherifnada March 24, 2023 13:27
@YowanR
Copy link
Contributor

YowanR commented Mar 25, 2023

@lazebnyi Please take a look at the latest comments and let's aim to wrap this up on Monday, please 🙏

@lazebnyi
Copy link
Collaborator Author

@YowanR thanks. It look like I already fixed few day ago) - #24421 (comment)

@YowanR
Copy link
Contributor

YowanR commented Mar 27, 2023

Looks like this is ready to be merged! Can we please proceed? @lazebnyi

@lazebnyi lazebnyi merged commit 97ba93c into master Mar 27, 2023
@lazebnyi lazebnyi deleted the lazebnyi/comapre-version-tool branch March 27, 2023 22:26
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.

4 participants