Mark release as beta #73

ahilles107 · 2014-07-11T16:06:35Z

Fix for #71.

How it work?

new option is added - label. It have 3 choices (RC, beta, alpha, none). If none is choosen then new version will be without label.

example workflow:

Current version: 0.0.0
type: p
label: a
Next Version: 0.0.1-alpha

Current version: 0.0.1-alpha
type: p
label: a
Next Version: 0.0.1-alpha2

Current version: 0.0.1-alpha2
type: p
label: b
Next Version: 0.0.1-beta

Current version: 0.0.1-beta
type: p
label: n
Next Version: 0.0.1

Current version: 0.0.1
type: p
label: n
Next Version: 0.0.2

Current version: 0.0.b
type: m
label: b
Next Version: 1.0.0-beta

Any help with this is welcome.

lsmith77 · 2014-07-11T17:14:12Z

src/Liip/RMT/Version/Generator/SemanticGenerator.php

+        if (isset($options['beta'])) {
+            $beta = $options['beta'];
+        }
+        else {


else should be on the previous line

lsmith77 · 2014-07-11T17:17:24Z

@jeanmonod can you have a look?

ahilles107 · 2014-07-11T17:21:11Z

I'm working on better solution discussed in in issue #71, and about else in new - i did it like this because this is standard in current library code.

ahilles107 · 2014-07-11T21:15:13Z

@acrobat, @jeanmonod, @lsmith77 - this should be much better, i don't know if it's easy to understand for every one. Help with code organization (if(){}else{}) would be great.

acrobat · 2014-07-12T13:26:34Z

Looks good! 👍

But with this change it should be different

Current version: 0.0.1-alpha
type: p
label: a
Next Version: 0.0.1-alpha1

In this case you should go from 0.0.1-alpha to 0.0.1-alpha2 because a label with no digit behind it, is always -_label_1

I'm going to take a look at the if/else code for label checking!

ahilles107 · 2014-07-12T13:43:02Z

@acrobat and this works like you said. My description was wrong.

ahilles107 · 2014-07-12T13:44:45Z

Test It in action please.

acrobat · 2014-07-12T13:50:19Z

oh ok, great! Yes i'm going to take a checkout of this branch and test it!

acrobat · 2014-07-12T14:21:28Z

I've just use this branch version of RMT and it works good, alpha and beta get the correct numbers behind it if multiple versions of label!

But there is are some problems with the RC label.

if you create a 0.0.1-RC version and you want to create a 0.0.1-RC2 you get the error that the tag 0.0.1-RC already exists, so it does not increment the RC label number
After a RC release the command RMT current still gives the previous version (version before the RC release)

Maybe it's a good idea to add some tests for the new label option and versions with labels

ahilles107 · 2014-07-12T14:47:31Z

Looks like wrong regex for version. I will look at It. I will try also with tests.

lsmith77 · 2014-07-14T08:06:29Z

src/Liip/RMT/Version/Generator/SemanticGenerator.php

+        if (isset($options['label'])) {
+            $label = $options['label'];
+        }
+        else {


else needs to be on the previous line :)

lsmith77 · 2014-07-14T08:07:37Z

can't you also use https://github.com/vierbergenlars/php-semver for parsing?

ahilles107 · 2014-07-14T08:12:49Z

@lsmith77 i will fix cs with next commit + missing test, I use vierbergenlars/php-semver for versions comparison.

About rc label problem - it must be wiath small letters, vierbergenlars/php-semver don't recognize 'RC' properly - works perfectly with 'rc' - so question is - should i leave it with small letters or try to find how to fix it with big letters?

acrobat · 2014-07-14T08:25:26Z

I think it's ok to use lowercase rc, it looks like it doesn't matter much because symfony tags it's pre-release (alpha, beta, rc) with all uppercase. So it's just a standard you agree on i think!

ahilles107 · 2014-07-14T08:45:04Z

Hey @acrobat @lsmith77, looks like all tests are passed (new one also), all works ok also for me, cs is fixed. Please review.

I still think that this if(){}else{} can be done better - of you agree then help me with this, if you think that this what we have now is ok then please merge it.

lsmith77 · 2014-07-14T09:41:08Z

src/Liip/RMT/Version/Generator/SemanticGenerator.php

+                $patch = substr($patch, 0, strpos($patch, "-"));
+
+                return implode(array($major, $minor, $patch), '.').'-'.$label.$labelVersion;
+            } else {


the following code does not need to be in an else, since the if statement before ends with a return

that being said .. seems like the end of the if statement could be refactored to no longer contain duplicate code in the else clause

beta_releases add missing option

ahilles107 · 2014-07-14T10:56:10Z

@lsmith77, @acrobat - rebased and cleaned.

lsmith77 · 2014-07-14T11:07:49Z

src/Liip/RMT/Version/Generator/SemanticGenerator.php

+                    // if label is new clear version
+                    if ($label !== $oldLabel) {
+                        $labelVersion = false;
+                    } else {


this should be an elseif now

ahilles107 · 2014-07-14T20:43:44Z

@acrobat it's ready to merge.

acrobat · 2014-07-14T21:09:27Z

great, nice work! But i think we should ping @lsmith77 @jeanmonod for the merge

takeit · 2014-07-15T07:43:44Z

👍

Mark release as beta

ahilles107 · 2014-07-15T08:50:32Z

Thanks for help guys!

…the label management. Most of the users were currently not using the label system, so forced them to always answer 'none' could be painful. To activate it on your project (or a dedicated branch) just add 'allow-label' in your config

jeanmonod · 2014-07-28T08:57:44Z

Hi guys,

First thanks you all for taking care of this. I was in holidays those last weeks, so not very present on the subject.
I just push one more commit to add your feature as an option and not by default. Everything is available under a new version 1.1.0

acrobat · 2014-07-28T12:25:23Z

👍 thanks @jeanmonod!

lsmith77 reviewed Jul 11, 2014
View reviewed changes

lsmith77 reviewed Jul 14, 2014
View reviewed changes

provide a way to add label for release

fa3d07e

beta_releases add missing option

lsmith77 reviewed Jul 14, 2014
View reviewed changes

simplify if/else, use rc php-semver release

c3d27ca

ahilles107 mentioned this pull request Jul 14, 2014

Update vierbergenlars/php-semver when stable version will be released #74

Closed

lsmith77 added a commit that referenced this pull request Jul 15, 2014

Merge pull request #73 from ahilles107/beta_releases

202a8a0

Mark release as beta

lsmith77 merged commit 202a8a0 into liip:master Jul 15, 2014

ahilles107 mentioned this pull request Jul 15, 2014

Add support for -alpha, -beta, -RC version tags #71

Closed

This was referenced Aug 6, 2014

3.0 stable release vierbergenlars/php-semver#34

Closed

Updated semver to stable release, fixes #74 #76

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mark release as beta #73

Mark release as beta #73

ahilles107 commented Jul 11, 2014

lsmith77 Jul 11, 2014

lsmith77 commented Jul 11, 2014

ahilles107 commented Jul 11, 2014

ahilles107 commented Jul 11, 2014

acrobat commented Jul 12, 2014

ahilles107 commented Jul 12, 2014

ahilles107 commented Jul 12, 2014

acrobat commented Jul 12, 2014

acrobat commented Jul 12, 2014

ahilles107 commented Jul 12, 2014

lsmith77 Jul 14, 2014

lsmith77 commented Jul 14, 2014

ahilles107 commented Jul 14, 2014

acrobat commented Jul 14, 2014

ahilles107 commented Jul 14, 2014

lsmith77 Jul 14, 2014

lsmith77 Jul 14, 2014

ahilles107 commented Jul 14, 2014

lsmith77 Jul 14, 2014

ahilles107 Jul 14, 2014

ahilles107 commented Jul 14, 2014

acrobat commented Jul 14, 2014

takeit commented Jul 15, 2014

ahilles107 commented Jul 15, 2014

jeanmonod commented Jul 28, 2014

acrobat commented Jul 28, 2014

Mark release as beta #73

Mark release as beta #73

Conversation

ahilles107 commented Jul 11, 2014

lsmith77 Jul 11, 2014

Choose a reason for hiding this comment

lsmith77 commented Jul 11, 2014

ahilles107 commented Jul 11, 2014

ahilles107 commented Jul 11, 2014

acrobat commented Jul 12, 2014

ahilles107 commented Jul 12, 2014

ahilles107 commented Jul 12, 2014

acrobat commented Jul 12, 2014

acrobat commented Jul 12, 2014

ahilles107 commented Jul 12, 2014

lsmith77 Jul 14, 2014

Choose a reason for hiding this comment

lsmith77 commented Jul 14, 2014

ahilles107 commented Jul 14, 2014

acrobat commented Jul 14, 2014

ahilles107 commented Jul 14, 2014

lsmith77 Jul 14, 2014

Choose a reason for hiding this comment

lsmith77 Jul 14, 2014

Choose a reason for hiding this comment

ahilles107 commented Jul 14, 2014

lsmith77 Jul 14, 2014

Choose a reason for hiding this comment

ahilles107 Jul 14, 2014

Choose a reason for hiding this comment

ahilles107 commented Jul 14, 2014

acrobat commented Jul 14, 2014

takeit commented Jul 15, 2014

ahilles107 commented Jul 15, 2014

jeanmonod commented Jul 28, 2014

acrobat commented Jul 28, 2014