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

feat(rest): first implementation #150

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat(rest): first implementation #150

wants to merge 2 commits into from

Conversation

Mte90
Copy link

@Mte90 Mte90 commented Jul 8, 2022

Ref: #26

For an example to test it https://github.com/Mte90/berlindb-rest/

I will update the example when will be merged https://github.com/berlindb/wordpress-example

Copy link
Collaborator

@JJJ JJJ left a comment

Choose a reason for hiding this comment

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

I've read through this, and I love that this is doable in only a few hundred lines of code, most of which are docs.

I wish that some of what you had to do here wasn't necessary (redefining schemas and tables and arrays) and I have some ideas about them, but not the time to write them all down here and now.

Eventually, I will have some changes to suggest, but until then I wanted to send encouraging thoughts and appreciation for you taking this on. It's super, super awesome! 🎂

@Mte90
Copy link
Author

Mte90 commented Jul 11, 2022

I was thinking too that some redefine like for the table etc wasn't necessary but this library just extend various classes and they are not connected each other so those data are not shared.
So in the meantime the patch let everything to work and those changes require some internals changes in BerlinDB that I think it's not the case to discuss here (to keep the focus on the REST stuff, maybe open a ticket for that?).

During the development of this integration I saw this issue in that PR:

  • my naming capability and documentation skills are not the best one
  • I don't know so much core internals to expose other data on the endpoints but I think that maybe release the support just for a subset of stuff is not a problem to see how works etc
  • the read function needs a little bit of love to improve it
  • I didn't added anything for escaping/sanitization as I see that BerlinDB already does but I am not sure...

@Mte90
Copy link
Author

Mte90 commented Jul 18, 2022

I was forgetting a little things, I just configured the doc comments for 3.0.0 release.

I still waiting for a review as I am curious for feedback :-D

@Mte90
Copy link
Author

Mte90 commented Jul 21, 2022

In the meantime I am using my fork in a plugin I am working on, so I have another test field.

@JJJ
Copy link
Collaborator

JJJ commented Sep 7, 2022

Haven't forgotten about this. 👋

Just been busy working on other 3.0 changes to hopefully make your work easier to merge/adopt. 🍕

@Mte90
Copy link
Author

Mte90 commented Sep 7, 2022

Thanks I just used BerlinDB in this period #23 as per that ticket and I think that the example need improvements until the documentation is not out there.

@Mte90
Copy link
Author

Mte90 commented Nov 8, 2022

just a ping

@mircobabini
Copy link

Can't wait to see this merged too. If you need some help to run tests or everything ✋🏻✋🏻✋🏻

@Mte90
Copy link
Author

Mte90 commented Jun 27, 2023

ping

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