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

Drop temporary migration code #18694

Merged

Conversation

AdrienClairembault
Copy link
Contributor

Checklist before requesting a review

  • I have performed a self-review of my code.

Description

This code was supposed to help developers work on different versions of the native forms feature without resetting their database too often.
In the end we still reset our databases all the time and this become more work to maintain when adding new feature so we should probably drop it now even if the database schema is not yet final for this feature.

@trasher
Copy link
Contributor

trasher commented Jan 9, 2025

Well... This is the way to proceed globally. I'm not against that, even if I do not always reset a fresh database (that means lost of all configuration, adn I often prefer to keep it).

I'm not against a change, but this should be clear we no longer use that way to proceed (maybe should we drop a line in developer docs anyway).

@trasher
Copy link
Contributor

trasher commented Jan 9, 2025

Also, I guess we must keep at least a par tof migrations, so users using released alphas can upgrade (dunno if removed code affects that).

@AdrienClairembault
Copy link
Contributor Author

I'm not against a change, but this should be clear we no longer use that way to proceed

I feel like this is the opposite, it's always been this way: database migration are expected to work correctly only when you migrate from one official version to another.
If you migrate from dev/alpha/beta, sure it will work most of the times but you risk having an incomplete database especially for big features that were developed over many months.

I remember there was a lot of support tickets with weird databases issues during the GLPI 10 migration because of this...
Maybe we could forbid database migration from unofficial versions unless the environnement is development ?

I agree that is should not be the case and that we can do a lot better than that but it requires reworking the ways our migrations work (multiple individuals migrations per version + down/up instructions) and that wont be done for GLPI 11 (I think @cedric-anne already has plans for this btw, probably for GLPI 12 then).

Copy link
Member

@cedric-anne cedric-anne left a comment

Choose a reason for hiding this comment

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

OK for this one, but it is preferable to always have migrations that permits to anyone to upgrade from a previous nightly, without having to erase the whole DB or to fix it manually.

Copy link
Contributor

@trasher trasher left a comment

Choose a reason for hiding this comment

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

As discussed in our internal chat, OK for me aswell

@trasher trasher merged commit 7349c9d into glpi-project:main Jan 13, 2025
9 checks passed
@AdrienClairembault AdrienClairembault deleted the delete-temporary-migration-code branch January 13, 2025 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants