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

Derive Copy and Clone for LayerTypes #670

Merged
merged 2 commits into from
Sep 22, 2023
Merged

Conversation

schneems
Copy link
Contributor

@schneems schneems commented Sep 8, 2023

I want to be able to inject LayerType into a struct instead of needing to hard code it. Currently this can be done but isn't ergonomic because LayerTypes is not clone, but it is serializable and deserializable and all values inside are copy. That allows us to hack around the issue currently:

struct InjectLayerTypes {
    visibility: LayerTypes,
	// ...
}

Imply Layer for InjectLayerTypes {
    // ...

    fn types(&self) -> LayerTypes {
        LayerTypes {
            launch: self.visibility.launch,
            build: self.visibility.build,
            cache: self.visibility.cache,
        }
    }

	// Or

    fn types(&self) -> LayerTypes {
        // Hack around LayerType not implementing `Clone`
        let toml_string =
            toml::to_string(&self.visibility).expect("LayerTypes can be serialized to toml");
        let layer_types: LayerTypes =
            toml::from_str(&toml_string).expect("Serialized data can be deserialized");

        layer_types
    }

This could be used to allow building of more generic interfaces. For example, in #598 I am trying to introduce a generic struct for use in setting environment variables. One gap in this implementation is that it assumes build, launch, cache should be true. It (or another struct) could instead make no assumptions and accept LayerTypes as an injectable value.

@schneems schneems marked this pull request as ready for review September 8, 2023 15:16
@schneems schneems requested a review from a team as a code owner September 8, 2023 15:16
Copy link
Member

@Malax Malax left a comment

Choose a reason for hiding this comment

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

👍🏻 on the change itself, we should also implement Copy. After the changelog is fixed this is 🟢 from my side.

CHANGELOG.md Outdated Show resolved Hide resolved
libcnb-data/src/layer_content_metadata.rs Outdated Show resolved Hide resolved
I want to be able to inject `LayerType` into a struct instead of needing to hard code it. Currently this can be done but isn't ergonomic because `LayerTypes` is not clone, but it is serializable and deserializable and all values inside are copy. That allows us to hack around the issue currently:

```
struct InjectLayerTypes {
    visibility: LayerTypes,
	// ...
}

Imply Layer for InjectLayerTypes {
    // ...

    fn types(&self) -> LayerTypes {
        LayerTypes {
            launch: self.visibility.launch,
            build: self.visibility.build,
            cache: self.visibility.cache,
        }
    }

	// Or

    fn types(&self) -> LayerTypes {
        // Hack around LayerType not implementing `Clone`
        let toml_string =
            toml::to_string(&self.visibility).expect("LayerTypes can be serialized to toml");
        let layer_types: LayerTypes =
            toml::from_str(&toml_string).expect("Serialized data can be deserialized");

        layer_types
    }
```

This could be used to allow building of more generic interfaces. For example, in #598 I am trying to introduce a generic struct for use in setting environment variables. One gap in this implementation is that it assumes build, launch, cache should be true. It (or another struct) could instead make no assumptions and accept LayerTypes as an injectable value.
CHANGELOG.md Outdated Show resolved Hide resolved
@edmorley edmorley changed the title Derive Clone for LayerTypes Derive Copy and Clone for LayerTypes Sep 22, 2023
@edmorley edmorley merged commit c7b2b6a into main Sep 22, 2023
4 checks passed
@edmorley edmorley deleted the schneems/layer-type-clone branch September 22, 2023 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants