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

Get Back Meaningful Message When "latest" Version Already Reached #987

Closed
bugalot opened this issue Jun 14, 2020 · 15 comments
Closed

Get Back Meaningful Message When "latest" Version Already Reached #987

bugalot opened this issue Jun 14, 2020 · 15 comments

Comments

@bugalot
Copy link
Contributor

bugalot commented Jun 14, 2020

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 😄.

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
Copy link
Member

goetas commented Jun 14, 2020

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

@bugalot
Copy link
Contributor Author

bugalot commented Jun 14, 2020

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

@Gemorroj
Copy link

Gemorroj commented Jun 15, 2020

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

@goetas
Copy link
Member

goetas commented Jun 15, 2020

UPD: use --allow-no-migration

yes

@akondas
Copy link

akondas commented Jun 17, 2020

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
Copy link

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
Copy link
Member

goetas commented Jun 18, 2020

#988 has been merged

@goetas
Copy link
Member

goetas commented Jun 19, 2020

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

@goetas goetas closed this as completed Jun 19, 2020
@akondas
Copy link

akondas commented Jun 19, 2020

Thanks @goetas 🍻

@goetas
Copy link
Member

goetas commented Jun 20, 2020

Related to this conversation : #1012 (comment)

@BonBonSlick
Copy link

[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
Copy link
Member

goetas commented Jun 20, 2020

See #1015, that should make this behavior compatible with the previous 2.x version

@goetas
Copy link
Member

goetas commented Jun 20, 2020

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

@BonBonSlick
Copy link

@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
Copy link
Member

goetas commented Jul 2, 2020

@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 #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants