Skip to content

Removing the delegate lookup feature from PSR-11#854

Merged
weierophinney merged 2 commits into
php-fig:masterfrom
moufmouf:remove_delegate_lookup
Dec 31, 2016
Merged

Removing the delegate lookup feature from PSR-11#854
weierophinney merged 2 commits into
php-fig:masterfrom
moufmouf:remove_delegate_lookup

Conversation

@moufmouf
Copy link
Copy Markdown
Contributor

Following discussion on the mailing list, there seems to be a consensus that the delegate lookup feature (which is actually a design pattern) might be worked on in another PSR (or may be not needed as a PSR at all as it is only a design pattern).

Copy link
Copy Markdown
Contributor

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

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

Looks good; I did not see any other spots where the feature is mentioned, and noted that you'd updated heading numbers correctly.

@weierophinney
Copy link
Copy Markdown
Contributor

weierophinney commented Dec 20, 2016

Not sure what I need to do to get pullapprove to pass? 👍 maybe? or LGTM?

Rejected with PullApprove

@michaelcullum
Copy link
Copy Markdown
Member

Just put 👍 as the message (nothing else).

@weierophinney
Copy link
Copy Markdown
Contributor

weierophinney commented Dec 20, 2016

👍

Approved with PullApprove

@weierophinney
Copy link
Copy Markdown
Contributor

Just put as the message (nothing else).

I couldn't find that information even in the pullapprove overview or features... Is there a way to make that more obvious in the future, @michaelcullum ?

@michaelcullum
Copy link
Copy Markdown
Member

michaelcullum commented Dec 20, 2016

@weierophinney GH keeps saying they'll open up approvals/reviews on their API, then pullapprove will use that. Until then it's actually regex defined in the repo which is why it's not on pull approve's site. You're welcome to modify it (https://github.com/php-fig/fig-standards/blob/master/.pullapprove.yml#L2) other than that I'm not sure where we could document this although I do try to generally inform people with commit access that it's there when they get given access.

@KorvinSzanto
Copy link
Copy Markdown
Contributor

@moufmouf Do you mind resolving the conflicts for this pull request?

@moufmouf
Copy link
Copy Markdown
Contributor Author

@KorvinSzanto Done, conflicts resolved.

@michaelcullum
Copy link
Copy Markdown
Member

As a general note, this will probably require a review period reset so you might want to get this merged ASAP if you are still planning on doing this on FIG 2.

@weierophinney weierophinney merged commit fd081f7 into php-fig:master Dec 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants