Skip to content

HEDNS: Adopt diff2 in compatibility mode#1887

Merged
tlimoncelli merged 3 commits into
masterfrom
tlim_diff2_hedns
Jan 2, 2023
Merged

HEDNS: Adopt diff2 in compatibility mode#1887
tlimoncelli merged 3 commits into
masterfrom
tlim_diff2_hedns

Conversation

@tlimoncelli
Copy link
Copy Markdown
Contributor

@tlimoncelli tlimoncelli commented Jan 1, 2023

Hi @rblenkinsopp:

In reference to replacing pkg/diff with the new pkg/diff2 (#1854)

I've introduced a compatibility mode to pkg/diff2 to make it easier for providers to adopt the new system. If you do a diff with "hide whitespace", you'll see the changes are minimal.

I've taken the liberty of providing this PR as a starting point.

Please run the integration tests and let me know the result. (If you're getting this, I probably don't have access to an account on this provider)

Please run:

    go test -v -verbose -provider HEDNS

and

    go test -v -verbose -provider HEDNS -diff2

They should both complete with no failures. Please fix any failures or loop me in for help.

If you are interested in porting to the new pkg/diff2/By*() functions, I'd be glad to accept a PR. However using the compatibility mode is sufficient if it passes all the tests.

Thanks and happy new years!!!

Tom
@tlimoncelli

@tlimoncelli tlimoncelli mentioned this pull request Jan 1, 2023
39 tasks
@rblenkinsopp
Copy link
Copy Markdown
Contributor

Hi @tlimoncelli, this PR passes with no test failures in both diff1 and diff2 modes, so happy to merge this :) I'll look at doing a proper Diff2 implementation shortly.

@tlimoncelli
Copy link
Copy Markdown
Contributor Author

Thanks for verifying! Happy new year!

@tlimoncelli tlimoncelli merged commit f8fd853 into master Jan 2, 2023
@tlimoncelli tlimoncelli deleted the tlim_diff2_hedns branch January 2, 2023 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants