+
Skip to content

Conversation

pileghoff
Copy link
Collaborator

@pileghoff pileghoff commented Jun 21, 2023

Note: this builds on #5 and #8, and will be rebased once those have been merged.

This removes the adapter and client, and merges the functionality of the client into the server.

You might want to check that my error handling is OK. That is one of the places where i still struggle in rust.

I got a little lazy with the examples and readme, sorry. I personally prefer a simple example instead of a long explanation, but i recognize the effort you put in and it pains me to remove it.

Feel free to add to the readme, if not, i will try to find the energy to do it.

Do say so, if you want me to split this up further or have any other feedback that could help me improve this PR 🏄🏽‍♀️

@sztomi
Copy link
Owner

sztomi commented Jun 23, 2023

I got a little lazy with the examples and readme, sorry. I personally prefer a simple example instead of a long explanation, but i recognize the effort you put in and it pains me to remove it.

TBH present me also prefers the more succinct example. The previous README over-explained unnecessary things (i.e. this is not the right place to learn how DAP works, there's an official documentation for that). This whole project was a distraction for me at the time and I focused on the smallest, most unimportant things possible (porting all the request and response types was a largely manual process!)

I think the sensible approach is to start with minimal docs and gradually extend them as friction is encountered.

@sztomi sztomi self-requested a review June 23, 2023 11:01
@sztomi
Copy link
Owner

sztomi commented Jun 23, 2023

I'll wait for your other PR to land and this one to be rebased, but on first look it looks really good - I'm humbled and thankful. Please consider adding a CONTRIBUTORS.txt and list your PRs with your name/username/email (whichever you prefer), similar to protobuf's CONTRIBUTORS.txt. Also, I'd be happy to add you as a maintainer to this repo (no commitment, just making it easier for you if you wish contribute more in the future). You can reach me on discord as tamas3441 if you'd like to chat.

@pileghoff
Copy link
Collaborator Author

Glad you like it 💯
I will add a PR with a CONTRIBUTORS.txt once this lands 👍🏽

Also, I'm impressed that your distractions look like this! If I ever use coding as a distraction, it usually looks a lot more sloppy!

@pileghoff pileghoff force-pushed the Remove-client-and-adapter branch from f69ecce to e45f0d6 Compare June 28, 2023 18:18
@pileghoff pileghoff force-pushed the Remove-client-and-adapter branch from e45f0d6 to b061893 Compare June 28, 2023 18:22
@pileghoff pileghoff marked this pull request as ready for review June 28, 2023 18:22
Copy link
Owner

@sztomi sztomi left a comment

Choose a reason for hiding this comment

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

Just small nits and this is ready to go

@pileghoff pileghoff requested a review from sztomi July 3, 2023 15:22
Copy link
Owner

@sztomi sztomi left a comment

Choose a reason for hiding this comment

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

Thank you so much 🎉

@sztomi sztomi changed the title Remove client and adapter, to create a simpler API Remove Client and Adapter to create a simpler API Jul 3, 2023
@sztomi sztomi merged commit cd603b1 into sztomi:main Jul 3, 2023
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.

2 participants

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载