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

com_installer version compare for database schema fixed #2146

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@Hackwar
Member

Hackwar commented Oct 5, 2013

Here is the bug report: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32179

I don't know if this needs to be fixed in the fix() task of the installer, too.

@sovainfo

This comment has been minimized.

sovainfo commented on a07bea3 Oct 5, 2013

Why the change in functionality with schemaVersion? When it is different it should be an error. That means the condition should be != 0

This comment has been minimized.

Owner

Hackwar replied Oct 5, 2013

If the version of the current schema is higher than the version of Joomla, it should in general be ok. The other way around is normally the problem.

This comment has been minimized.

sovainfo replied Oct 5, 2013

It is not ok when there are structural differences. Old scripts can't deal with changed structure.

This comment has been minimized.

mbabker replied Oct 5, 2013

If the schema version is higher than the reported Joomla version, the only way to get there is if the user downgraded the file structure without handling the database. It'll only be an issue with having a newer database schema if there are fields that are NOT NULL with no defaults.

Either way, using a version_compare statement is more correct than what's in place now.

This comment has been minimized.

sovainfo replied Oct 5, 2013

Just as !=0 is more correct than == -1 !!!

This comment has been minimized.

Owner

Hackwar replied Oct 6, 2013

I'll fix it. :-)

@Hackwar

This comment has been minimized.

Member

Hackwar commented Dec 5, 2013

So these two are still open? No wonder people get missing update notices... Could we please merge this? Thanks.

@Bakual

This comment has been minimized.

Contributor

Bakual commented Dec 5, 2013

Same message here as in the other PR: Not enough testers logged in the tracker. And the one test doesn't even say which PR got tested (2.5 or 3.2?).

@infograf768

This comment has been minimized.

Member

infograf768 commented Feb 13, 2014

Please redo towards staging (2.5.x one now merged). Thanks

@mbabker mbabker closed this in b382d19 Feb 17, 2014

@mbabker

This comment has been minimized.

Member

mbabker commented Feb 17, 2014

Merged manually to staging.

Bakual added a commit to Bakual/joomla-cms that referenced this pull request May 12, 2014

@Hackwar Hackwar deleted the Hackwar:inst_version_comp branch Dec 10, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment