Skip to content

Conversation

@misakstvanu
Copy link
Contributor

Hi,
clonning the instance with every configuration change is very expensive and not necessary in context of this library, I propose just setting the property and returning original object.

This also fixes Flagsmith.php line 183

    public function withCache(?CacheInterface $cache): self
    {
        if (is_null($cache)) {
            $this->with('cache', null); <-- result of this expression is not used anywhere (someone probably missed the clonning)
        }

        return $this->with(
            'cache',
            new Cache($cache, $this->cachePrefix, $this->timeToLive)
        );
    }

@matthewelwell
Copy link
Contributor

I'm not sure why CI hasn't run on this PR but I'm definitely keen to see the tests run given this is a change to quite a fundamental part of the library. Can you make sure that you're up to date with the latest from the main branch in the target repository?

@misakstvanu
Copy link
Contributor Author

I synced the repos.

@matthewelwell matthewelwell merged commit 631bc64 into Flagsmith:main Aug 28, 2024
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