Closed adrexia closed 5 years ago
Cool, thanks for the hard work!
Since we are changing class name and have the namespace, I would be inclined to remove RESTfulAPI
out of all class names, since it already is in the namespace. That would make everything even neater... what do you think?
@adrexia We'd be keen to see this change incorporated into a release but I agree with @colymba that the namespace makes the RESTfulAPI
part of the class name redundant. There is a possibility that I might be able to help out with that if you're short of time to make the changes.
I'd also like to reiterate, thanks for your work in making this update. :)
@robingram if you could, that would be great! We're a bit snowed under right now, so haven't had time to look at this since it was first done.
@adrexia I'm in the process but I came across a number of issues. The main problem was that it couldn't work for namespaced models. I managed to get it operational for a basic unauthenticated use case last week but I want to make sure that the test suite is green.
I hope to get it done this week but realistically I only have Thursday to work on it.
Hi @colymba, @adrexia. I'm finally close to having this ready to go. I just want to review the history and probably squash a few commits because currently there are 33. :)
How would you prefer me to handle the PR? I guess there are a couple of options. I could create a PR to dnadesign
that would then update this or create a whole new PR here to replace this one.
I'm leaning towards the latter because there are a couple of changes that I think it would be good for @colymba to look through and that might just cause problems for DNA if they are already in their fork.
@robingram do what's easiest for you, we don't mind too much. :)
@robingram on dnadesign is fine. Thx!
worked continued here https://github.com/colymba/silverstripe-restfulapi/pull/93
Updates to make this module compatible with silverstripe 4