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.

Sign up for GitHub

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

Jump to bottom

[#266] Set minimum-stability to stable and added docs. #667

Merged
merged 1 commit into from
May 28, 2024
Merged

[#266] Set minimum-stability to stable and added docs. #667

merged 1 commit into from
May 28, 2024

Conversation

AlexSkrypnyk
Copy link
Collaborator

Closes #266

@AlexSkrypnyk AlexSkrypnyk marked this pull request as draft May 17, 2024 07:44
@AlexSkrypnyk AlexSkrypnyk force-pushed the feature/266-11x-min-stability branch from d416363 to 425a118 Compare May 17, 2024 07:58
AlexSkrypnyk
AlexSkrypnyk commented May 17, 2024
View reviewed changes
composer.json Outdated
@@ -15,6 +15,7 @@
"drupal/core-composer-scaffold": "^11@alpha",
"drupal/core-recommended": "^11@alpha",
"drush/drush": "^13@beta",
"chi-teck/drupal-code-generator": "4.x-dev#e61b917 as 3.5.0",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package was updated to Symfony 7, but there is no release.

Considering that the whole 11.x branch is "early preview", I think this should be okay to have like this.

Once Drupal 11 is in beta and all these packages have their releases, we can clean it up.

@AlexSkrypnyk AlexSkrypnyk force-pushed the feature/266-11x-min-stability branch from 425a118 to c0ac5e8 Compare May 17, 2024 08:00
AlexSkrypnyk
AlexSkrypnyk commented May 17, 2024
View reviewed changes
composer.json Outdated
@@ -10,6 +10,7 @@
}
],
"require": {
"chi-teck/drupal-code-generator": "4.x-dev#e61b917 as 3.5.0",
Copy link
Collaborator Author

@AlexSkrypnyk AlexSkrypnyk May 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package was updated to Symfony 7, but there is no release.

Considering that the whole 11.x branch is "early preview", I think this should be okay to have like this.

Once Drupal 11 is in beta and all these packages have their releases, we can clean it up.

I've created an issue - maybe we can wait for a couple of days before merging this one.

Copy link

@simesy simesy May 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need to add as 3.5.0 unless another package explicitly needs that version. If you are using composer require ... it won't work because it checks stability. But if you just add it manually to the composer.json it works fine. This is a feature afaik.

To test, I updated it locally to be this and ran composer update.

 "chi-teck/drupal-code-generator": "4.x-dev",

I don't know if pinning it to a commit hash is good... if the package maintainers happen to rewrite history on the dev branch then composer will stop building?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it worked for you because you probably did not use the dev version of core.

Drush 13.x requires 4.x-dev for Symfony 7 compatibility https://github.com/drush-ops/drush/blob/13.x/composer.json#L36C3-L36C57

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chi-teck/drupal-code-generator is now released. waiting for Drush to start using it at alpha+ version level.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drush-ops/drush#6010

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it worked for you because you probably did not use the dev version of core.

Sorry I isn't sinking in why this project needs to use the dev version of core.

@AlexSkrypnyk AlexSkrypnyk mentioned this pull request May 17, 2024
Closed
@AlexSkrypnyk AlexSkrypnyk marked this pull request as ready for review May 17, 2024 08:04
@AlexSkrypnyk AlexSkrypnyk self-assigned this May 17, 2024
@AlexSkrypnyk AlexSkrypnyk added D11 PR: Needs review Pull request needs a review from assigned developers labels May 17, 2024
@AlexSkrypnyk AlexSkrypnyk requested review from leymannx and webflo May 17, 2024 08:04
simesy
simesy reviewed May 20, 2024
View reviewed changes
composer.json
@@ -36,7 +37,7 @@
"url": "https://asset-packagist.org"
}
],
"minimum-stability": "dev",
"minimum-stability": "alpha",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this change.

@AlexSkrypnyk AlexSkrypnyk force-pushed the feature/266-11x-min-stability branch 4 times, most recently from cc60531 to f5a4641 Compare May 20, 2024 19:59
@leymannx leymannx changed the title [#266] Set minimum-stability to true and added docs. [#266] Set minimum-stability to stable and added docs. May 21, 2024
leymannx
leymannx approved these changes May 21, 2024
View reviewed changes
Copy link
Collaborator

@leymannx leymannx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!! Very clear documentation, I like. I'm also in favor of waiting for new code generator release instead of getting the alias committed. But when there's a follow-up issue so we don't forget about it I'm also okay with it in the meanwhile.

@AlexSkrypnyk AlexSkrypnyk force-pushed the feature/266-11x-min-stability branch from f5a4641 to fca7327 Compare May 21, 2024 07:17
@AlexSkrypnyk
Copy link
Collaborator Author

@leymannx
We now have an issue with Drush (drush-ops/drush#6010). I'm going to wait for a couple of days before merging the alias (ugly).

@AlexSkrypnyk AlexSkrypnyk added BLOCKED Issue or pull request is blocked UPSTREAM Issue or pull request is related to an upstream project labels May 21, 2024
@AlexSkrypnyk AlexSkrypnyk force-pushed the feature/266-11x-min-stability branch from fca7327 to 2b64fbc Compare May 28, 2024 01:29
@AlexSkrypnyk AlexSkrypnyk removed BLOCKED Issue or pull request is blocked UPSTREAM Issue or pull request is related to an upstream project labels May 28, 2024
@AlexSkrypnyk AlexSkrypnyk merged commit 806a6bf into 11.x May 28, 2024
10 checks passed
@AlexSkrypnyk AlexSkrypnyk deleted the feature/266-11x-min-stability branch August 14, 2024 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simesy simesy simesy left review comments

@leymannx leymannx leymannx approved these changes

@webflo webflo Awaiting requested review from webflo

Assignees

@AlexSkrypnyk AlexSkrypnyk

Labels
D11 PR: Needs review Pull request needs a review from assigned developers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AlexSkrypnyk @simesy @leymannx