Skip to content

HETZNER: Adopt diff2 in compatibility mode#1888

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

HETZNER: Adopt diff2 in compatibility mode#1888
tlimoncelli merged 3 commits into
masterfrom
tlim_diff2_hetzner

Conversation

@tlimoncelli
Copy link
Copy Markdown
Contributor

Hi @das7pad:

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 HETZNER

and

    go test -v -verbose -provider HETZNER -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 all tests pass.

Thanks and happy new years!!!

Tom
@tlimoncelli

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

@das7pad das7pad left a comment

Choose a reason for hiding this comment

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

LGTM. I've tested a similar patch (das7pad@4bcb80e) earlier today and the integration tests passed with both the baseline flags and -diff2.

Happy new year!

@tlimoncelli
Copy link
Copy Markdown
Contributor Author

Thanks!

Wow! You're diff is more concise. I don't know why I didn't think of doing it that way. For consistency with the others I'll merge my PR.

@tlimoncelli tlimoncelli merged commit 4083a0c into master Jan 2, 2023
@tlimoncelli tlimoncelli deleted the tlim_diff2_hetzner branch January 2, 2023 13:27
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