search

modmore / VersionX

Resource & Element Versioning Extra for MODX Revolution (supports 2.2 and up). Extends the core in a future-proof manner to keep copies of every change to resources, templates, template variables, chunks, snippets and plugins.
https://modmore.com/extras/versionx/
40 stars 20 forks source link

return false if it's not saved #66

Closed goldsky closed 6 years ago

goldsky commented 10 years ago

When we used your class, we could not detect whether the saving succeeded or not, because the saving function itself is wrapped in a condition

        if($this->checkLastVersion('vxResource', $version, $this->debug)) {
            return $version->save();
        }

This particular method should return false at the end, to mark that there is no newResourceVersion is made because no condition is met

adamwintle commented 10 years ago

:+1:

ientfong commented 10 years ago

:+1:

reisharanty commented 10 years ago

:+1:

manjarb commented 10 years ago

:+1:

Mark-H commented 10 years ago

Let me guess, all those +1's are monogon? ;)

Mark-H commented 10 years ago

I cannot merge this as is as this breaks consistency with the various new*Version methods that still return true. FALSE also shouldn't be uppercase in terms of coding style, but that's nitpicking.

If you want to update the other new*Version methods as well I'll merge this in. Otherwise I'll address this in the next release as I do agree it should return false if it did not write a new version.

Mark-H commented 6 years ago

I've created an issue for updating new*Version methods to return an appropriate boolean.