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

API Rescue Master Branch PR: Only expose public extension methods #10457

Conversation

GuySartorelli
Copy link
Member

@GuySartorelli GuySartorelli commented Aug 23, 2022

Rescues #7752 and #7879

Also strongly types Extensible and CustomMethods as per https://github.com/silverstripeltd/product-issues/issues/583 - easier to do it here while I'm already changing things than to do it as a follow-up PR, though I can happily remove that commit if the reviewer doesn't want them in the same PR.

Depends on

Parent issues

@GuySartorelli GuySartorelli force-pushed the pulls/5/rescue-master-extensions-expose-public branch from c3930f9 to 11d8f23 Compare August 23, 2022 01:01
@GuySartorelli GuySartorelli mentioned this pull request Aug 23, 2022
5 tasks
@GuySartorelli GuySartorelli changed the title API Only expose public extension methods API Rescue Master Branch PR: Only expose public extension methods Aug 23, 2022
@GuySartorelli GuySartorelli force-pushed the pulls/5/rescue-master-extensions-expose-public branch 2 times, most recently from 2dafffb to 0a62ce7 Compare August 23, 2022 01:10
@emteknetnz
Copy link
Member

PR is currently combination of 2x different things

  • rescuing a master branch commit
  • adding strong typing

These are two different things and should be done in separate PRs

Cheers

@GuySartorelli GuySartorelli force-pushed the pulls/5/rescue-master-extensions-expose-public branch from 0a62ce7 to bab1ed7 Compare August 24, 2022 03:26
@GuySartorelli
Copy link
Member Author

Strong-typing commit has been removed.
I probably won't be creating a new PR for it though since I'd have to wait for this to be merged and create a new PR from that point to avoid merge conflicts - we'll just have to live without.

Damian Mooyman added 2 commits August 24, 2022 15:31
API Enable `extend*` methods to handle ->extend()
API Remove Extensible::constructExtensions()
@GuySartorelli GuySartorelli force-pushed the pulls/5/rescue-master-extensions-expose-public branch from bab1ed7 to f2211d6 Compare August 24, 2022 03:31
@emteknetnz emteknetnz merged commit 250a75b into silverstripe:5 Aug 29, 2022
@emteknetnz emteknetnz deleted the pulls/5/rescue-master-extensions-expose-public branch August 29, 2022 07:09
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.

None yet

2 participants