This repository has been archived by the owner on Feb 11, 2019. It is now read-only.
.virtPHP/ core structure rework (attempt 2) #36
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…file that we can migrate current functionality into.
…lass for interface with that object.
…lder by default instead of current directory.
…ne used throughout the rest of the code.
…one tracked in the environments.json file.
… check in the get method.
…ot relying on current directories any longer.
…adding the new env to the list. Also extended the clone to name to a full path so it will be created in the env folder.
…env name and to add new entry.
…s and files, and checking for specific env.
…ent object and also expanded to full path on creating env.
…d clone tests until we can get that working again.
…n false where needed.
Conflicts: src/Virtphp/Workers/Cloner.php
Found destroy needs to remove it's editing of the environments.json file and move to environmentFile object. Will make this change and update the PR. |
…ronmentFile object.
…he environment file object.
Work done, should be ready to review. |
I reviewed this with @jwoodcock at hacknashville, I'm good with it. |
|
||
class Command extends ConsoleCommand | ||
{ | ||
public $envFile; | ||
|
||
public $outpout; |
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.
This appears to be a misspelling. Let's fix it and make sure it's not misspelled elsewhere.
Aside from my nitpicks, everything looks good to me. |
…a getEnvFile method instead of set so the interface is cleaner and more declarative.
@ramsey 's nitpicks are fixed. |
ramsey
added a commit
that referenced
this pull request
May 14, 2014
.virtPHP/ core structure rework (attempt 2)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As explained in the email I sent on 4/15/14, listed below, I've reworked how we are creating environments to set the default location to be inside the .virtphp/envs/ folder now.
Also, I've added a number of tests to bring us back up to decent code coverage, not 100%.
The one thing I could not get done as of yet, is the virtphp activate {env name} command. Though I think we are close to getting this worked out.
The big work is the creation of the EnvironmentFile object to handle all communication to and from the environments.json.
EMAIL--------------------------------------------------------
However, because we do not want them adding this folder to their projects, we're moving to do an install script, and virtphp are not project specific, I propose we create all env folders in the ~/.virtphp folder.
This will help keep peoples systems cleaner and can allow us to put an activate command into the virtphp application that simply activates environments.
Something like: virtphp activate {env name}
We should also make environment name unique for a number of reasons.
I'll be happy to tackle all this work for v1 if approved.