Get Back Meaningful Message When "latest" Version Already Reached

bugalot commented 4 years ago

Bug Report

Q	A
BC Break	yes/no
Version	3.0.0

Summary

When launching a migration on a fully migrated platform (i.e. on "latest" version), the message changed from 3.0.0-alpha1 to 3.0.0-beta1/stable.

Before we had a nice and clean "[WARNING] Already at latest version" which was perfect.

Now we have "[ERROR] The version "latest" couldn't be reached, you are at version xxx" which surprised a bit our developer community.

We provide a Pull Request in order to get back the more meaningful message. It would be great if you guys could have a look at it and maybe integrate it :smile:.

It might look like a detail but we feel that providing nice and meaningful is essential. It seems that the message change happened in this commit. It is just a "loss" we would like to see back again.

Current behavior

When a platform is at "latest" version, launching a new migration produces the following output:

[ERROR] The version "latest" couldn't be reached, you are at version xxx

How to reproduce

Launch a successful migration process, then run it again.

Expected behavior

When a platform is at "latest" version, launching a new migration produce the following output:

[WARNING] Already at latest version

goetas commented 4 years ago

It is not a BC break, console messages are not covered by BC promise

bugalot commented 4 years ago

Agreed, it was more for convenience! Up to you to decide !

Gemorroj commented 4 years ago

@goetas but why it's now ERROR (not WARNING)? it breaks our ci. UPD: use --allow-no-migration

goetas commented 4 years ago

UPD: use --allow-no-migration

yes

akondas commented 4 years ago

It is not a BC break, console messages are not covered by BC promise

Console messages not, but this is not only message text change but also exist code. In my opinion, however, this is a change worth at least adding to the changelog.

fmarchalemisys commented 4 years ago

The message is not only misleading, it is wrong too.

The latest version was reached without error! It Just did not happen during this run.

There is no need to alarm the user about it.

goetas commented 4 years ago

https://github.com/doctrine/migrations/pull/988 has been merged

goetas commented 4 years ago

988 has been merged (that improves the wording in the message)
/993 has been merged (that explain the --allow-no-migration command)

So I'm closing this.

I see that multiple people are disagreeing with the current implementation, this are the possibilities:

use the --allow-no-migration (exit code is successful and a warning message is not an error)
find a backward compatible way to make this work as some of you expect

akondas commented 4 years ago

Thanks @goetas :beers:

goetas commented 4 years ago

BonBonSlick commented 4 years ago

[ERROR] The version "latest" couldn't be reached, you are at version "0"


{
    "type": "project",
    "license": "proprietary",
    "require": {
        "php": "^7.1.3",
        "ext-ctype": "*",
        "ext-iconv": "*",
        "ext-json": "*",
        "cron/cron-bundle": "^2.4",
        "drenso/phan-extensions": "^2.5",
        "friendsofsymfony/rest-bundle": "^2.7",
        "nelmio/api-doc-bundle": "^3.5",
        "nelmio/cors-bundle": "^2.0",
        "phan/phan": "^2.5",
        "ramsey/uuid": "^3.9",
        "sensio/framework-extra-bundle": "^5.5",
        "symfony/browser-kit": "4.4.*",
        "symfony/console": "4.4.*",
        "symfony/dotenv": "4.4.*",
        "symfony/flex": "^1.3.1",
        "symfony/framework-bundle": "4.4.*",
        "symfony/http-client": "4.4.*",
        "symfony/lock": "4.4.*",
        "symfony/monolog-bundle": "^3.5",
        "symfony/options-resolver": "4.4.*",
        "symfony/orm-pack": "^1.0",
        "symfony/security-bundle": "4.4.*",
        "symfony/serializer": "4.4.*",
        "symfony/serializer-pack": "^1.0",
        "symfony/validator": "4.4.*",
        "symfony/yaml": "4.4.*"
    },
    "require-dev": {
        "dama/doctrine-test-bundle": "^6.3",
        "doctrine/doctrine-fixtures-bundle": "^3.3",
        "phpdocumentor/reflection-docblock": "^4.0",
        "phpunit/phpunit": "^8.5",
        "roave/security-advisories": "dev-master",
        "symfony/maker-bundle": "^1.14",
        "symfony/phpunit-bridge": "^5.0"
    },
    "config": {
        "preferred-install": {
            "*": "dist"
        },
        "sort-packages": true
    },
    "autoload": {
        "psr-4": {
            "App\\": "src/"
        }
    },
    "autoload-dev": {
        "psr-4": {
            "App\\Tests\\": "tests/"
        }
    },
    "replace": {
        "paragonie/random_compat": "2.*",
        "symfony/polyfill-ctype": "*",
        "symfony/polyfill-iconv": "*",
        "symfony/polyfill-php71": "*",
        "symfony/polyfill-php70": "*",
        "symfony/polyfill-php56": "*"
    },
    "scripts": {
        "auto-scripts": {
            "cache:clear": "symfony-cmd",
            "assets:install %PUBLIC_DIR%": "symfony-cmd"
        },
        "post-install-cmd": [
            "@auto-scripts"
        ],
        "post-update-cmd": [
            "@auto-scripts"
        ]
    },
    "conflict": {
        "symfony/symfony": "*"
    },
    "extra": {
        "symfony": {
            "allow-contrib": false,
            "require": "4.4.*"
        }
    }
}

goetas commented 4 years ago

See https://github.com/doctrine/migrations/pull/1015, that should make this behavior compatible with the previous 2.x version

goetas commented 4 years ago

@BonBonSlick please avoid posting questions not related to this ticket. If you have questions please open a dedicated ticket.

BonBonSlick commented 4 years ago

@goetas why it was not related? I found solution, after composer update config file messed

doctrine_migrations:
    migrations_paths:
        dir_name: '%kernel.project_dir%/src/Migrations' // BAD
        'DoctrineMigrations': '%kernel.project_dir%/migrations' // BAD
        'DoctrineMigrations': '%kernel.project_dir%/src/Migrations' // GOOD!

goetas commented 4 years ago

@BonBonSlick this ticket was about that message being displayed when there are no other migrations to execute (but migrations were loaded correctly), and that is what https://github.com/doctrine/migrations/pull/988 solved. The issue you have described (and solved by your self) was a configuration error in your YAML files that was preventing you from finding the migrations to load from disk.

doctrine / migrations