search

webrtc-sdk / libwebrtc

A C++ wrapper for binary release, mainly used for flutter-webrtc desktop (windows, linux, embedded).
MIT License
378 stars 77 forks source link

I added doc strings to 7 files #67

Closed markdicks closed 1 year ago

markdicks commented 1 year ago

There were 3 files that I added doc strings for improved code readability and maintainability.

markdicks commented 1 year ago

I would love to work on more files and add more doc strings if you keen, just comment and let me know

cloudwebrtc commented 1 year ago

Yes, I'd love to accept PRs for these awesome docs, please go ahead.

markdicks commented 1 year ago

Awesome thank you

markdicks commented 1 year ago

My bad I closed it thinking it was merged, apologies

cloudwebrtc commented 1 year ago

I can merge now, but if you want to continue adding other files, I will review them later

markdicks commented 1 year ago

In about 10-12 hours I will be back to continue with the rest of the files in the libwebrtc/src folder. Hopefully finish those, for now you can merge if you comfortable with that.

markdicks commented 1 year ago

Then do lemme know which files you'd like me to focus on then I will do those as well.

cloudwebrtc commented 1 year ago

In terms of priority, the API documentation in libwebrtc/include is the most needed to add docs, you can start here If you want, most APIs can refer to https://developer.mozilla.org/en-US/docs/Web /API/WebRTC_API is the calling method,

markdicks commented 1 year ago

The two deletions are coming from the 'include/rtc_audio_device.h' file, there were two comments that I removed, about 4 words each. I removed them because it seemed covered in the new doc strings I added in both deletions. Hope you don't mind that minor change. Your code still remains unaffected.

On that I have done 4 files in the folder you suggested and went in depth for all four. I hope to continue later on today, you can merge as this is the first load of doc strings. I hope to do 4 big batches like this. Hope you are satisfied. Talk soon xD

cloudwebrtc commented 1 year ago

lgtm!

cloudwebrtc commented 1 year ago

@markdicks Can you create a PR in branch if possible? This saves you from triggering an update to the PR when rolling the master branch forward. For example change

image

to docs/api-docs-for-xxxx

markdicks commented 1 year ago

Okay will do from now on, thank you

markdicks commented 1 year ago

Good day, hope you doing well today. Sorry for my lack of knowledge, I am not sure what is meant when you say create a PR in the branch. Please do let me know, and then furthermore I did not make any changes to the master branch on my side after you merged but it still tells me that I am 9 commits ahead. Sorry about this, I tried asking a work colleague about it but even they was not sure what to do.