Lampho branch - Php version on Laravel-Zero - Initial, progressive upgrade to 5.8

[ijpatricio]

hey @andrewmile

So I was able to start the porting, and left out the recurring runtime option config change in a starting point.

All the rest is working, and left a file, ROADMAP.md that you should go about, left many thought and indications there!

Im at 12% now, will use the rest to scaffold a little more, the interactive options, and if all is found agreed, just small tweaks and tests missing.

Can't wait to hear your thoughts!

[ijpatricio]

I think, best to start, would be cloning the fork on ijpatricio:ijpatricio-lampho-58
And analyse on your editor.

ROADMP.md has instructions and insights.

8% battery :) Configuration options.. I left them going to the looks of how it could be, but then they just get back to initial screen.

Form here, similar like the one on 5.7, i was thinking making a contract and classes for options, that would change the runtime configs, (perform check is a plus), they would run, and get back to the display of current configuration.

Now it's missing, because all the options will be different, but let's make them show as you mentioned before, showing the evaluation (instead of "false"), and a friendly description.

Phhheeew this was long, hope you made it through!! :)

Let me know your thoughts!!

[andrewmile]

hey @ijpatricio , nice work! I just pulled down the branch and gave it a quick run through. It works and everything looks good so far. I'll look over this more over the next few days as I have extra time but will give a more detailed review by Friday.

Thanks again! :)

[ijpatricio]

hey @andrewmile !

Sure thing! Glad I can help! :)

Let me know any further feedback after you review.

I'm having an idea for implementation of the options (now without php-console-menu), just not sure if I should push it before you review??

Thanks! :)

[andrewmile]

Hey @ijpatricio , I've been reviewing the updates today and have some more feedback below for some things that I think will improve the workflow. Please let me know what you think!

Default Editor

The editor attempts to open with PhpStorm by default. Let’s change the config.editor option to default to false so it can just be set in the user’s config.php.

Default node

Let’s also set the default config.node to false. This will make the default behavior consistent with the current version of Lambo.

Options for making or editing config.php and after.php

The current version of Lambo supports options for make-config, edit-config, make-after, edit-after. The make commands will give a “Config already exists." message if the file already exists and the edit commands will create the file if it doesn’t already exist and then open in the editor set in the config or use Vim as a fallback. What do you think about adding the following commands to keep this same behavior?
- make:config
- make:after
- edit:config
- edit:after

Make customization prompt optional

What do you think about hiding the customization prompt behind a flag so it will only run if the flag is passed in? I don’t necessarily want to get rid of the customization prompt because I think it can be useful but when I run lambo new app my default expectation is for it to just run its normal process so I mentally pause at the customization prompt.

Adding the following to the signature would mean if I want to run with customization I could just run lambo new app -c.

{--c|custom : Customize config options.}

Then with the above, you could wrap a condition around the PromptForCustomization call like below:

if ($this->option('custom') {
    $this->action(PromptForCustomization::class);
}

[andrewmile]

Hi @ijpatricio , here's a few more comments below:

CD into new directory

After the new app installation finishes I am left in the same directory I was in when I ran the command. Can you add an extra action in the handle method to cd into the newly created directory?

Config.php overrides

I created a new file at ~/.lambo/after.php with the following contents in order to open the project in Sublime but nothing happened. Do I need to format this file differently?

<?php

return [
    'editor' => 'subl'
];

After installation

The current version of Lambo runs an after shell command if it is present at ~/.lambo/after. The default stub is below:

#!/usr/bin/env bash

# Install additional composer dependencies as you would from the command line.
# echo "
# Installing Composer Dependencies
# "
# composer require tightenco/mailthief tightenco/quicksand

# To copy standard files to new lambo project place them in ~/.lambo/includes directory.
# echo "
# Copying Include Files
# "
# cp -R ~/.lambo/includes/ $PROJECTPATH

# To add a git commit after given modifications
# echo "
# Committing after modifications to Git
# "
# git add .
# git commit -am "Initialize Composer dependencies and additional files."

In the previous PR that there was an after.php file that got merged with config/lambo/after.php but I no longer see that file in this branch. Is that still a work in progress? I wonder if it would be just as easy to check for the presence of ~/.lambo/after and run it through $this->shell(). That would allow for backwards compatibility for anyone who has a bunch of things going on in that file.

[ijpatricio]

Hey @andrewmile !! Glad to hear you're enjoying so far! 👍

So, I changed the workflow already (run is the default, -c to config).
Started adding tests.
Added TODOS, where I still did not implement. I would add them here, but that would be crazy to understand. They are all in app/Commands/NewCommand.php

Now i have conditions to work more, at least on the TODOS...And testing.
Not sure when and where you want to chime in, so let me know!

Also shared my feedback about yours 🙌So here goes...

Default Editor
The editor attempts to open with PhpStorm by default. Let’s change the config.editor option to default >to false so it can just be set in the user’s config.php.

Done.

Default node
Let’s also set the default config.node to false. This will make the default behavior consistent with the >current version of Lambo.

Good point. Done.

Options for making or editing config.php and after.php
The current version of Lambo supports options for make-config, edit-config, make-after, edit-after. >The make commands will give a “Config already exists." message if the file already exists and the edit >commands will create the file if it doesn’t already exist and then open in the editor set in the config or >use Vim as a fallback. What do you think about adding the following commands to keep this same >behavior?

• make:config
• make:after
• edit:config
• edit:after

I would see this consistently with Lambo now being interactive, so all in one go!
Think with me:

• We can lambo new automator to just start the installer (default, or last config, if already set).
• Or we can lambo new automator -c to customise.
  • Now that I have -c and options to decide what to do, why not:
    • Config as we wish...
    • Save config, and save after may be options to choose!
    • If destination file already exists, just warn "Want to override existing config?" In this case, we may even copy the existing file, adding a timestamp, so that no data would be lost inadvertently. Keep last 3 copies.

What do you think?

I'm also ok with making that 4 other commands, what do you think?

Also, speaking of these files. IIRC, files are saved to ~/.lambo
Nowadays, apps like Valet, Yarn and more, are using ~/.config/app so we could have ~/.config/lambo

Make customisation prompt optional
What do you think about hiding the customisation prompt behind a flag so it will only run if the flag is >passed in? I don’t necessarily want to get rid of the custosization prompt because I think it can be >useful but when I run lambo new app my default expectation is for it to just run its normal process so I >mentally pause at the customization prompt.

Adding the following to the signature would mean if I want to run with customization I could just run >lambo new app -c.

{--c|custom : Customize config options.}
Then with the above, you could wrap a condition around the PromptForCustomization call like below:

if ($this->option('custom') {
$this->action(PromptForCustomization::class);
}

This is done already, help me to figure if I understood correctly

lambo new automator -c

Should:
1 - Display current config (maybe we just want to check), Press c to config, or r to run
2 - Display immediately the config (without knowing how it was) ?

Right now is doing option 1.

CD into new directory
After the new app installation finishes I am left in the same directory I was in when I ran the command. >Can you add an extra action in the handle method to cd into the newly created directory?

Yeah, so this will be a struggle. I will have to double check with a friend, just not to give you the bad news right away 😞Well, maybe there's a way. laravel official installer doesnt do it, also laravel-zero does not. I will research a little further and ask opinion, maybe you can do the same?

Config.php overrides
I created a new file at ~/.lambo/after.php with the following contents in order to open the project in >Sublime but nothing happened. Do I need to format this file differently?

'subl' ];

(...)

In the previous PR that there was an after.php file that got merged with config/lambo/after.php but I no >longer see that file in this branch. Is that still a work in progress? I wonder if it would be just as easy to >check for the presence of ~/.lambo/after and run it through $this->shell(). That would allow for >backwards compatibility for anyone who has a bunch of things going on in that file.

Yeah, this was in 5.7 branch, but with the port to 5.8 being a little progressive, the workflow of those 2 files is still to do. That's a quick one.

Waiting your feedback 😀

[ijpatricio]

Hey @andrewmile

I went ahead, and implemented the interactive option, that in previous version with 5.7 had console-menus, and now it's with native laravel choice, which makes it easier to test.

So please clone from the fork, and try it out. Also added tests. composer test or ./vendor/bin/phpunit

Now it should be very easy to add the missing interactive options, but because I don't know if i will have more time, I decided to commit this and notify you.

Have a good one!

[andrewmile]

Hey @ijpatricio , this is making some nice progress :)

I like the change you made of defaulting to running the command right away and adding the -c flag.

I would see this consistently with Lambo now being interactive, so all in one go!
Think with me:

We can lambo new automator to just start the installer (default, or last config, if already set).
Or we can lambo new automator -c to customise.
Now that I have -c and options to decide what to do, why not:
Config as we wish...
Save config, and save after may be options to choose!
If destination file already exists, just warn "Want to override existing config?" In this case, we may even copy the existing file, adding a timestamp, so that no data would be lost inadvertently. Keep last 3 copies.
What do you think?

I'm also ok with making that 4 other commands, what do you think?

Yeah I like the idea of being able to edit and save the config from the command line. I think we could start out with that option and see if it feels like that provides all of the functionality. If it does, there may not be a need to actually edit the file directly. So lets go with the edit and save from command line option and then evaluate from there.

Also, speaking of these files. IIRC, files are saved to ~/.lambo
Nowadays, apps like Valet, Yarn and more, are using ~/.config/app so we could have ~/.config/lambo

I like this idea, lets go for it 👍

This is done already, help me to figure if I understood correctly

lambo new automator -c

Should:
1 - Display current config (maybe we just want to check), Press c to config, or r to run
2 - Display immediately the config (without knowing how it was) ?

Right now is doing option 1.

I prefer option 1 as well.

Yeah, so this will be a struggle. I will have to double check with a friend, just not to give you the bad news right away 😞Well, maybe there's a way. laravel official installer doesnt do it, also laravel-zero does not. I will research a little further and ask opinion, maybe you can do the same?

I'll work on this and see if I can come up with something.

[andrewmile]

@ijpatricio I just created a PR (https://github.com/ijpatricio/lambo/pull/2) to the ijpatricio-lampho-58 branch of your Lambo fork in order to add a final action to CD into the new project directory. Once that PR is merged, the functionality will be present in this PR as well.

I played around with several different options here but landed on creating a change-directory.sh shell script that could be called from PHP with shell_exec. The new action is ChangeDirectory and gets called after all of the other install actions.

[ijpatricio]

Hey Andrew! Cool news! I'll try it very soon! Time hasnt been abundant, but as we're so close, we got to push this forward. Already made one more menu option, now its easier.. Thought we could split that.. Im answering, even on mobile, so you dont wait longer, but this next days ill push more work, and then we do a followup, if u agree, with the purpose to find the closest way for a first version. Happy coding! A sexta, 26/04/2019, 19:02, Andrew Morgan <notifications@github.com> escreveu:

…

@ijpatricio <https://github.com/ijpatricio> I just created the PR below to the ijpatricio-lampho-58 branch of your Lambo fork in order to add a final action to CD into the new project directory. Once that PR is merged, the functionality will be present in this PR as well. I played around with several different options here but landed on creating a change-directory.sh shell script that could be called from PHP with shell_exec. The new action is ChangeDirectory and gets called after all of the other install actions. PR: ijpatricio#2 <https://github.com/ijpatricio/lambo/pull/2> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#68 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGGTKY5ALIICLXYRHBMHWZ3PSM7TRANCNFSM4HCNG4UA> .

[andrewmile]

Hey @ijpatricio , no worries - things have been pretty hectic for me lately as well but I'm going to take some time today to review what is remaining on this :)

[ijpatricio]

Hello Andrew! Hope you're doing well! So, in about 2-3 weeks I will have again a little "time to breathe".. And would love to pickup on this and finish it! How's things on your end? Patricio

…

On Fri, May 10, 2019 at 3:42 PM Andrew Morgan ***@***.***> wrote: Hey @ijpatricio <https://github.com/ijpatricio> , no worries - things have been pretty hectic for me lately as well but I'm going to take some time today to review what is remaining on this :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#68 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGGTKY4LMUKANJWVMMPACGTPUWCV3ANCNFSM4HCNG4UA> .

[andrewmile]

Hi Patricio! Thanks for checking in on this :)

Things are going well on my end. I created a new branch from your PR a while back and have made updates to am/laravel-zero-implementation as I have had time available. Feel free to pull it down and have a look at where things are at. By the time you start working on it again I'll make a WIP PR with a checklist of outstanding items on this branch. That way if you want to PR into this new branch we can make updates there and work off of the same list. How does that sound to you?

[mattstauffer]

So.... this has grown out of control. I say this with love and care but it's so heavy I just can't get a handle on it.

I'm going to start a brand-new install from scratch. I'm going to build it out simply, bit by bit, and I welcome pull requests into it--even if the PRs are direct copies from this branch. I would love that! But I need to get this back to a place where I have a handle on it.

Sorry if this feels like any time was wasted. I hope not!

[ijpatricio]

Eheheh it sure did :)

@mattstauffer

There is absolutely nothing to be sorry about. Personally, it was a great experience, given I have almost no experience in open source, and it was fun. More than that, if you are willing to start from scratch, and accept PR's, count me in!

So please let me know when you do so.

@andrewmile

Time really flies! Hopefully with this new momentum the project will grow!