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

Mechanism to rename config fields and retain backwards compatibility #4631

Closed
mitchellh opened this issue Jan 5, 2025 · 2 comments · Fixed by #5329
Closed

Mechanism to rename config fields and retain backwards compatibility #4631

mitchellh opened this issue Jan 5, 2025 · 2 comments · Fixed by #5329
Milestone

Comments

@mitchellh
Copy link
Contributor

Now that we're a public project I'm more sensitive to maintaining backwards compatibility. I don't have any official policy around it [yet] except to "try to retain it unless necessary to break."

We now have a concrete use case for renaming a config field in #4403. We want to rename background-blur-radius to background-blur.

To do this, I want to introduce a mechanism to src/cli/args.zig so that we can rename fields and have the old one map to the new one.

@mitchellh mitchellh added this to the 1.0.2 milestone Jan 5, 2025
@jcollie
Copy link
Collaborator

jcollie commented Jan 5, 2025

I think that a comptime-generated struct pub fn Deprecated(comptime renamed_to: []const u8) type could do the job.

@pluiedev
Copy link
Contributor

pluiedev commented Jan 13, 2025

Maybe we should instead have versioned config migrations that allow us to have old fields with old types that can be converted into new fields with new types (and chain them all together, of course)? This way we can more robustly support older versions when the migration steps required involve more than just a simple renaming. Doing this in other languages would be really annoying but with Zig comptime I think it's a lot more managable

/// Config as in 1.0.1
pub const ConfigV1_0_1 = {
	// fields
    @"background-blur-radius": u8 = 0,
    @"adw-toast": AdwToast = ...,
    @"hypothetical-field-to-remove": u32 = 0,
};

/// Config as in 1.1.0
pub const ConfigV1_1_0 = Migration(
    // keys to remove
    &.{
        .@"background-blur-radius",
        .@"adw-toast",
        .@"hypothetical-field-to-remove",
    }, 
    
    // keys to add
    struct {
        @"background-blur": BackgroundBlur = .false,
        @"app-notifications": AppNotifications = .{ .clipboard_copy = true },
        @"sparkling-new-field": ?[]const u8 = null,
    },
    
    struct {
        // Old will be a subset of fields matching the keys specified in first argument
        fn migrate(old: ConfigV1_1_0.Old) ConfigV1_1_0.New {
            return .{
                .@"background-blur" = .{ .radius = old.@"background-blur-radius" },
                .@"app-notifications" = old.@"adw-toast",
                .@"sparkling-new-field" = null,
            };
        }
    }.migrate,
);

mitchellh added a commit that referenced this issue Jan 23, 2025
Fixes #4631

This introduces a mechanism by which parsed config fields can be renamed
to maintain backwards compatibility. This already has a use case --
implemented in this commit -- for `background-blur-radius` to be renamed
to `background-blur`.

The remapping is comptime-known which lets us do some comptime
validation. The remap check isn't done unless no fields match which
means for well-formed config files, there's no overhead.

For future improvements:

- We should update our config help generator to note renamed fields.
- We could offer automatic migration of config files be rewriting them.
- We can enrich the value type with more metadata to help with
  config gen or other tooling.
mitchellh added a commit that referenced this issue Jan 23, 2025
…#5329)

Fixes #4631

This introduces a mechanism by which parsed config fields can be renamed
to maintain backwards compatibility. This already has a use case --
implemented in this commit -- for `background-blur-radius` to be renamed
to `background-blur`.

The remapping is comptime-known which lets us do some comptime
validation. The remap check isn't done unless no fields match which
means for well-formed config files, there's no overhead.

For future improvements:

- We should update our config help generator to note renamed fields.
- We could offer automatic migration of config files be rewriting them.
- We can enrich the value type with more metadata to help with config
gen or other tooling.
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 a pull request may close this issue.

3 participants