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

Add a reference to setFromDb() #506

Closed
wants to merge 1 commit into from

Conversation

peterhauke
Copy link
Contributor

There does not appear to be any reference to this function in the documentation, however it appears in the setupListOperation() and setupCreateOperation() when creating a CrudController, so I added a description (adapted from setFromDb() in AutoSet.php)

There does not appear to be any reference to this function in the documentation, however it appears in the setupListOperation() and setupCreateOperation() when creating a CrudController, so I added a description (adapted from setFromDb() in AutoSet.php)
@karandatwani92 karandatwani92 added Priority: SHOULD documentation Improvements or additions to documentation Size: XS 1 hour labels Sep 1, 2023
@karandatwani92
Copy link
Contributor

Hey, @tabacitu setFromDb() is missing in the CRUD API list.

Please review and merge.

Thanks @peterhauke

@tabacitu
Copy link
Member

tabacitu commented Sep 1, 2023

Thanks for the note @peterhauke !

Actually @karandatwani92 we don't want people using setFromDb() - it's deprecated. So instead, let's remove any mentions of it from the docs, please 🙏

Why? Because setFromDb() isn't good-enough in the first place. It doesn't properly guess column&field types. And because of a way it was constructed, we can never ever make updates to it. Any updates we do make... are a breaking change to everybody who's using it. That's why we've deprecated it.

PS. I don't know if it already has a @deprecated note on the docblock, in the code. If it does not, please do that too @karandatwani92

@peterhauke
Copy link
Contributor Author

peterhauke commented Sep 1, 2023

Thanks @tabacitu

I had a look at setFromDb() in autoset.php and there does not appear to be a deprecated note there.

I had a look at setupListOperation() in crud-controller.stub and there is no mention of it being deprecated there either.

setFromDb() is currently being included in the code when a new crud controller is created.

Sorry for asking, but is there another function to use instead of setFromDb() ?

@karandatwani92
Copy link
Contributor

Yes, Not appropriate to @deprecate as it's in crud-controller.stub .

@tabacitu please give directions.

@promatik
Copy link
Contributor

promatik commented Jan 7, 2024

@karandatwani92 it's true the setFromDb is in the stubs, it should be removed from there, anyway, in the between we can keep it as an example, but developers will be warned that it is deprecated.
I'll close this one, let's move conversation here; Laravel-Backpack/CRUD#5419

@promatik promatik closed this Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation Priority: WON'T Size: XS 1 hour
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants