Skip to content

Conversation

@jrbarnard
Copy link
Contributor

@jrbarnard jrbarnard commented Oct 2, 2019

Feature to provide functionality requested in #799

Added under a separate command to be used alongside 'links' for only parked paths.

I left Site::getLinks as I wasn't sure if I should just replace it's usage with the newly added Site::getSites, I can do if that would be better.

Screen Shot 2019-10-02 at 12 32 04

@jrbarnard
Copy link
Contributor Author

If merged we'll need to add a PR to the Laravel valet docs

@drbyte
Copy link
Contributor

drbyte commented Oct 2, 2019

Nicely done.

I'd suggest we mark getLinks as deprecated with a docblock, mentioning to use getSites instead. And then update the call to it in the function above your new parked function.

(I'd get rid of it altogether, except that there's a tiny rare chance that someone's written an extension that would be broken. Then again, it's such a minor change that any extension could be easily and quickly fixed without creating crazy hardship. This isn't in a production environment!)

@jrbarnard
Copy link
Contributor Author

@drbyte Sounds good, I've marked as @deprecated for now.

@mattstauffer
Copy link
Collaborator

@jrbarnard This looks great. Would you mind fixing the merge resolution issue?

@jrbarnard
Copy link
Contributor Author

@mattstauffer No problem, I've updated my branch now to remove the conflict

@mattstauffer mattstauffer merged commit 21fb0a3 into laravel:master Oct 4, 2019
@mattstauffer
Copy link
Collaborator

Thanks @jrbarnard!

@mattstauffer
Copy link
Collaborator

OK, released in 2.5.0. @jrbarnard are you up to do that docs PR?

@jrbarnard
Copy link
Contributor Author

jrbarnard commented Oct 4, 2019 via email

@mattstauffer
Copy link
Collaborator

Geez, i dunno. 5.8 maybe?

@drbyte
Copy link
Contributor

drbyte commented Oct 4, 2019

I dunno what Taylor's preference is. Does he prefer 6.0 and then backport manually? Multiple PRs for each? 5.8 and then he'll merge fwd manually? I dunno.

This is where Valet's docs really diverge from Laravel's docs altogether, unfortunately.

@jrbarnard
Copy link
Contributor Author

I'll go for 5.8 and mention it on the PR so can change if needed. Cheers

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.

3 participants