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

[10.x] Prompts #46772

Merged
merged 31 commits into from
Jul 31, 2023
Merged

[10.x] Prompts #46772

merged 31 commits into from
Jul 31, 2023

Conversation

jessarcher
Copy link
Member

No description provided.

@jessarcher jessarcher marked this pull request as draft April 14, 2023 02:41
@jessarcher jessarcher force-pushed the prompts branch 3 times, most recently from a9451a6 to 31c75ec Compare April 19, 2023 07:29
Copy link
Member

@nunomaduro nunomaduro left a comment

Choose a reason for hiding this comment

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

src/Illuminate/Console/OutputStyle.php Outdated Show resolved Hide resolved
src/Illuminate/Console/OutputStyle.php Show resolved Hide resolved
@jessarcher jessarcher force-pushed the prompts branch 7 times, most recently from a906b34 to 886bf0e Compare July 28, 2023 07:00
@jessarcher jessarcher force-pushed the prompts branch 2 times, most recently from f149434 to 1cc5207 Compare July 28, 2023 07:06
Comment on lines +32 to +33
with:
fetch-depth: 0
Copy link
Member Author

Choose a reason for hiding this comment

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

I hate having to do this.

The issue is that laravel/prompts depends on illuminate/collections ^10.0, which creates a circular dependency. This would ordinarily be fine because the composer.json in this project specifies that it provides illuminate/collections at self.version. However, when doing a shallow clone in CI, none of the tags exist, so self.version resolves to the hash of the commit, which doesn't satisfy the dependency.

Alternatives are:

  • Make laravel/prompts depend on * instead of ^10.0 but remove the ability for composer to force a known compatible version.
  • Remove the dependency from laravel/prompts and rely on built-in PHP methods.
  • Add the top-level version key to the composer.json on this repo and maintain it going forward.
  • Add an extra step to CI on this project to automatically add the version key to composer.json just for the duration of the build, scraping it from Application.php. We'd need to be careful with any actions that make commits (e.g. update changelog) to make sure they don't include the change.

I went with this option as it felt the least hacky and problematic. The downside is that the clone is slower and bigger (an additional ~20MB of git history). If this isn't acceptable, I think I'd opt for removing collections from laravel/prompts.

Copy link
Member

Choose a reason for hiding this comment

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

If Illuminate\Support\Collection is only used for type-hint and $collect instanceof Collection I believe we can skip it from laravel/prompts as it shouldn't cause any issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

Prompts is using collections functionality in a few places, not just the types :(

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

Successfully merging this pull request may close these issues.

5 participants