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

Backward compatibility for chown, broken in 0.28.0 -> 0.29.0 #1551

Merged

Conversation

deblasis
Copy link
Contributor

This PR simply takes into account uid and gid settings in the template, introduced in 0.28.0 but replaced by user and group in 0.29.0, breaking compatibility.

We address the issue by giving precedence to the newer settings but setting the relevant properties upon type conversion if the legacy settings are provided

Uid (0.28.0) -> User (0.29.0)
Gid (0.28.0) -> Group (0.29.0)

Related #1541

This PR simply takes into account `uid` and `gid` settings in the template, introduced in 0.28.0 but replaced by `user` and `group` in 0.29.0, breaking compatibility.

We address the issue by giving precedence to the newer settings but setting the relevant properties upon type conversion if the legacy settings are provided

Uid (0.28.0) -> User (0.29.0)
Gid (0.28.0) -> Group (0.29.0)

Signed-off-by: Alessandro De Blasis <[email protected]>
@deblasis deblasis requested a review from a team March 16, 2022 18:41
@eikenb
Copy link
Contributor

eikenb commented Mar 16, 2022

Hey @deblasis, thanks for writing this and you are right in that it would be better to keep compatibility for now. Though I do need come up with a system for deprecating and removing things so I can add this (along with a few other things) to it.

Copy link
Contributor

@eikenb eikenb left a comment

Choose a reason for hiding this comment

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

Simple change with test coverage. 👍

@eikenb eikenb merged commit 007153f into hashicorp:master Mar 16, 2022
@deblasis
Copy link
Contributor Author

deblasis commented Mar 16, 2022

Hey @deblasis, thanks for writing this and you are right in that it would be better to keep compatibility for now. Though I do need come up with a system for deprecating and removing things so I can add this (along with a few other things) to it.

No worries! I am assuming that 1.0 is gonna be the version with these deprecations. Well, I guess it depends on how strictly you are following semantic versioning.
I noticed that there is at least another instance of legacy stuff that is still supported but that could be deprecated. Something around "connection timeout".

In the past I have used a simple markdown file named like the next major version where I listed all the legacy things that had to be deprecated. I still remember the folder name: sunsetting 🤭

It was descriptive text for documentation but it contained an items list with checkboxes that was parsed by a simple github action that would fail if you tried to merge a branch with a new major tag when the markdown file matching that version contained items left unchecked.

Not really a fancy system, quite manual and simple but it helped.
I am sure there are better ways, I am curious to find out what you come up with 😊

@eikenb eikenb modified the milestones: v0.29.0, v0.28.1 Mar 28, 2022
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