-
Notifications
You must be signed in to change notification settings - Fork 113
Improve the installation of -dev and -BETA versions #185
Changes from 11 commits
d58812a
94a1f98
4c26d2b
d67b4ad
d03938f
9b3cdc1
31765ca
b26b217
9e41335
165813c
f45a29a
3ea393a
6751701
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,6 +106,12 @@ public function provideSymfonyInstallationData() | |
'/.*Symfony 2\.5\.6 was successfully installed.*/', | ||
'/Symfony version 2\.5\.6 - app\/dev\/debug/', | ||
), | ||
|
||
array( | ||
'2.7-BETA1', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This version should fail while There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed. Fixed. Thanks. |
||
'/.*Symfony 2\.7\.0\-BETA1 was successfully installed.*/', | ||
'/Symfony version 2\.7\-BETA1 - app\/dev\/debug/', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to need an update too, doesn't it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm trying to do things fast ... but I do everything wrong :( Thanks for reviweing this! |
||
), | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could now pass a patch version and a dev suffix. I think both parts should be exclusive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dev suffixes are all rejected just later, so this is not a big deal IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but this does not only apply to the
-dev
suffix but also to-BETA
and-RC
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and beta versions are named 2.1.0-beta, not 2.1-beta
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm indeed, then we can move that part of the regex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed this regular expression by:
Please review it. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you forget to push the changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've pushed it .... and then reverted it. The correct beta version is
x.y.z-BETAn
, notx.y-BETAn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There would still be some versions that pass the regex but are not valid. Though actually the installer will fail with a meaningful message anyway later on. So I think it's okay if we just stick with this solution.