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

Next beta 🤔 #136

Merged
merged 72 commits into from
Dec 24, 2020
Merged

Next beta 🤔 #136

merged 72 commits into from
Dec 24, 2020

Conversation

jonsugar
Copy link
Collaborator

@jonsugar jonsugar commented Sep 22, 2020

@mattstauffer I think this is a reasonable chunk of fixes and new features to release as a beta. It should not be too big to review this time 😁 .

NOTE: the large file change count is due to the removal of \App\Commands\LamboCommand->makeAndInvoke(). You will likely be able to skip over most of the files during the review.

User facing changes

✨  Feature
Users can instruct Lambo to run database migrations. Migrations will also run automatically if the user has specified --inertia or --livewire as a Jetstream backed UI requires specific database tables.

# database migration support 🔥 
Lambo new my-side-hustle --migrate-db

✨  Feature
Lambo will give the user better feedback as it executes. In prior betas it was necessary to add the -v[vv] flag to see the commands being executed and whether they completed successfully (fixes #126).

Screenshot 2020-09-22 at 19 11 42

Under-the-hood changes

Outstanding Issues

For v1.0.0

Description Milestone
✨  Feature
Add --force option (#123)
v1.0.0beta4 (29-SEP-2020)
✨  Feature
Make opening in editor/browser configurable (#124)
v1.0.0beta4 (29-SEP-2020)
♻️  Refactor
Configuration improvements (#134, #119 and #109)
v1.0.0beta4 (29-SEP-2020)
🐛  Bug
Issues opening the project in VS Code (#127)
v1.0.0beta4 (29-SEP-2020)
📝  Docs
Update README.md docs to reference the PHP version (#111)
v1.0.0rc1 (06-OCT-2020)

For post release

Description Milestone
✨  Feature
Support Windows and Linux option (#117)
Post launch (TBA)
✨  Feature
Add a post execution summary (#108)
Post launch (TBA)
✨  Feature
Automatically add new options to ~/.lambo/config (#110)
Post launch (TBA)
✨  Feature
Add a --without-comments option (#85)
Post launch (TBA)
✨  Feature
Add new option lambo --interactive
This will show a menu for configuring Lambo execution (#115)
Post launch (TBA)
✨  Feature
Implement lambo edit-config --interactive
This will show a menu for setting defaults in ~/.lambo/config (#114)
Post launch (TBA)

jonsugar and others added 26 commits September 16, 2020 11:32
- when the --migrate-db flag is provided
- when either --inertia or --livewire is provided.
# Conflicts:
#	app/Actions/ConfigureFrontendFramework.php
#	app/Actions/ValidateConfiguration.php
#	builds/lambo
#	composer.json
#	composer.lock
#	readme.md
@jonsugar jonsugar marked this pull request as draft September 22, 2020 18:29
@jonsugar jonsugar changed the title Requested changes from code review and a few extra new bits. Next beta 🤔 Sep 22, 2020
ensure the default saved configuration stub is compatible with all the new and changed configuration introduced during the port to PHP

fixes #109
Set PHP 7.3 and Laravel Zero 8.0 as minimum version dependencies

fixes #137
@jonsugar jonsugar marked this pull request as draft October 5, 2020 17:17
@jonsugar jonsugar marked this pull request as ready for review October 9, 2020 18:39
@jonsugar
Copy link
Collaborator Author

jonsugar commented Oct 9, 2020

@mattstauffer As discussed, requesting a review.

Notes:
I'm fully aware that embedding, $newConfiguration, $removedConfigurationKeys, and $configurationVersion in in app/Actions/UpgradeSavedConfiguration.php is not the greatest but as we decided to push the refactor of all things Configuration to post-launch I felt it best for now.

Also, I needed a way for Lambo to check which version of the config file it last updated to. I wasn't sure whether to store the data in the saved config file its self or in a separate file. In the end, I chose a separate file ~/.lambo/.last_version_update.

@mattstauffer
Copy link
Member

@jonsugar Hey, how are we looking on this? Thanks!

@jonsugar
Copy link
Collaborator Author

@mattstauffer I've just seen this, I wasn't receiving emails so missed it—I've updated notifications. I'll take a look tomorrow but...

As far as I remember, I had completed all the tasks we had agreed on that were required for release. I think the branch is stable but since time has passed things have changed.

As you noted (and have since closed) we needed to update to a version of Laravel Zero that supports PHP8. I will merge this into my branch and test it.

Also, the Laravel installer no-longer uses Zips, just composer. This means there is literally no benefit to us using it–previously it was more performant. Perhaps now, we can just call composer ourselves and trap the console output like we do for other tools. Essentially, what we already do for the inertia/livewire installations.

Playing devil's advocate, we could also get rid of all our custom code for Jetstream installations and just use the official installer for all installation tasks. This means less to maintain for us at the expense of not being able to trap the console output and make things pretty–not my favorite option tbh

@mattstauffer
Copy link
Member

As far as I remember, I had completed all the tasks we had agreed on that were required for release. I think the branch is stable but since time has passed things have changed.

Woo! Reviewing now.

As you noted (and have since closed) we needed to update to a version of Laravel Zero that supports PHP8. I will merge this into my branch and test it.

👌

Also, the Laravel installer no-longer uses Zips, just composer. This means there is literally no benefit to us using it–previously it was more performant. Perhaps now, we can just call composer ourselves and trap the console output like we do for other tools. Essentially, what we already do for the inertia/livewire installations.

Fine by me!

Playing devil's advocate, we could also get rid of all our custom code for Jetstream installations and just use the official installer for all installation tasks. This means less to maintain for us at the expense of not being able to trap the console output and make things pretty–not my favorite option tbh

Same here. Unless the installer gets fancier and more complex over time.

@mattstauffer
Copy link
Member

Ahh, I had asked you to un-do the thing where you un-dependency-injected the console writer. No worries, I'll fix that myself.

@mattstauffer mattstauffer merged commit 7f46062 into tighten:main Dec 24, 2020
@mattstauffer
Copy link
Member

Thanks for all your hard work on this @jonsugar!

@jonsugar
Copy link
Collaborator Author

jonsugar commented Dec 26, 2020

Ahh, I had asked you to un-do the thing where you un-dependency-injected the console writer. No worries, I'll fix that myself.

@mattstauffer Yeah, but... https://github.com/tighten/lambo/projects/2#card-46721232

@jonsugar jonsugar deleted the jrs/1.0.0beta3 branch December 26, 2020 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants