-
Notifications
You must be signed in to change notification settings - Fork 2k
feat: Add builds next option
#9946
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
base: develop
Are you sure you want to change the base?
Conversation
michalsn
left a comment
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'm not a big fan of introducing next, as it will require an update that we will most likely forget to make.
Additionally, removing the instructions on how to adjust the composer file manually will leave people clueless about what to do.
I would agree to introducing something like allowing a version number to be specified, for example: 4.8, 4.9, etc., which would result in a proper, explicit change - but this parameter would need to come with a detailed explanation.
|
We have a workflow for the appstarter release. What if I replace the constant for task? Honestly, I do not know if anyone uses the dev version except me. If we continue, it is advisable to specify the next CI version with the release. Because the dev has the current number 4.7.0. In fact, it's 4.7.1 or 4.8.0. module-manager won't forgive this. |
If the version can be adjusted automatically for the next minor release, then I would accept this.
Yes, it's used.
Not sure about it. As it could lead to a situation where someone thinks they are using version ie, |
|
I understand, that's why I indicated the note in the manual. |
|
michalsn
left a comment
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.
Looks good.
admin/starter/builds
Outdated
| define('LATEST_RELEASE', '^4.0'); | ||
| define('LATEST_RELEASE', '^4.7'); | ||
| define('DEVELOP_BRANCH', 'dev-develop'); | ||
| define('NEXT_MINOR', '^4.8-dev'); |
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.
Is caret allowed by composer for dev versions? I think this should be 4.8.x-dev here and in the release script?
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 performed composer update and it did not cause any errors. I'll delete the symbol, it's more correct this way.
admin/prepare-release.php
Outdated
| $minor = $versionParts[0] . '.' . $versionParts[1]; | ||
|
|
||
| // Note: Major version will change someday (4.x..5.x) - update manually. | ||
| $nextVersion = $versionParts[0] . '.' . $versionParts[1] + 1; |
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.
$nextMinor should be more appropriate name here
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.
Done
admin/RELEASE.md
Outdated
| and `<version number="4.x.x">` | ||
| * Update **admin/starter/builds**: | ||
| * Set `define('LATEST_RELEASE', '^4.x')` | ||
| * Set `define('NEXT_MINOR', '^4.y-dev')`. |
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.
See previous comment.
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.
Done
admin/RELEASE.md
Outdated
| * Update **admin/starter/builds**: | ||
| * Set `define('LATEST_RELEASE', '^4.x')` | ||
| * Set `define('NEXT_MINOR', '^4.y-dev')`. | ||
| * If the major version changes, you need to manually change to `define('NEXT_MINOR', '^5.0-dev')`. |
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.
On a semver perspective, 5.0 is not really next minor but next major. Maybe add a note instead to define a NEXT_MAJOR constant when time comes?
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.
How will it work? If we create a v5.0, how do we determine that it 5.0-dev needs to be inserted and not 4.y-dev?
| Others | ||
| ====== | ||
|
|
||
| - **builds:** In the ``builds`` script (for ``codeigniter4/appstarter``), the ``next`` option has been added |
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.
Not an option but an argument (since no preceding dashes).
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.
Done
| ====== | ||
|
|
||
| - **builds:** In the ``builds`` script (for ``codeigniter4/appstarter``), the ``next`` option has been added | ||
| to switch ``4.7.x`` to the next minor version ``4.8-dev``. See :ref:`Latest Dev<switch-to-dev-version>`. |
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.
| to switch ``4.7.x`` to the next minor version ``4.8-dev``. See :ref:`Latest Dev<switch-to-dev-version>`. | |
| to switch ``4.7.x`` to the next minor version ``4.8.x-dev``. See :ref:`Latest Dev<switch-to-dev-version>`. |
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.
Why not 4.8-dev? composer successfully download the branch
| Note that this differs from the released user guide, and will pertain to the | ||
| develop branch explicitly. | ||
|
|
||
| .. note:: You should not rely on the version of the framework in your project |
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.
Off-topic for another PR, maybe? Should "unstable" versions have a suffixed -DEV on the version number? Like for develop branch it would be the next patch 4.7.1-DEV while 4.8 branch is 4.8-DEV? @michalsn ?
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.
Seems like version_compare() would handle this without a problem, so if we can automate this, then why not? Without automation, we would probably forget about it in a few weeks.
Description
Checklist: