Add a storeOriginalConfiguration method

[jonathanjfshaw]

Adds a helper method to avoid overwriting original configuration, that stores a value to the original configuration store only if no such value has previously been stored.

[jonathanjfshaw]

Fixes #195, from #182.

[jonathanjfshaw]

I've not tested it properly.

[jhedstrom]

Is there a reason to have a return value at all here? It doesn't appear to be used in this change...

[jonathanjfshaw]

There's no good reason; I was just a bit unhappy about silently ignoring what the calling code had requested. Probably I was just trying to be too clever. Shall I remove it?

…

On Mon, 27 Aug 2018 at 19:13, Jonathan Hedstrom ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/Drupal/Driver/Cores/Drupal8.php <#196 (comment)> : > + * Store the original value for a piece of configuration. + * + * @param string $name + * The name of the configuration. + * @param mixed $value + * The original value of the configuration. + * + * @return bool + * Whether or not $value is now stored as the original configuration value. + */ + protected function storeOriginalConfiguration($name, $value) { + if (!isset($this->originalConfiguration[$name])) { + $this->originalConfiguration[$name] = $value; + return TRUE; + } + return ($this->originalConfiguration[$name] === $value); Is there a reason to have a return value at all here? It doesn't appear to be used in this change... — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#196 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADWYQa3e4N8mV33HM6aD-NsGL8WeY4Nfks5uVDa1gaJpZM4WNqca> .

[jhedstrom]

Yeah, can you remove it and then update the docblock comment to make it clear that the value will only be set if it isn't already (and perhaps the reasoning for this)? If we need a return value in the future it can always be added I think.

[jonathanjfshaw]

Done

[jhedstrom]

Thanks!