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 inline doc comments to the Web SDK template #721

Merged
merged 3 commits into from
Jun 4, 2024

Conversation

Im-Madhur-Gupta
Copy link
Contributor

What does this PR do?

This PR adds inline comments to the web SDK template. These comments will be visible in IDEs.

Test Plan

I generated the SDK from the web template after making changes into it using example.php. This verified that the comments were showing up correctly.

Related PRs and Issues

Have you read the Contributing Guidelines on issues?

Yes, I have.

@Im-Madhur-Gupta
Copy link
Contributor Author

I have added the comments by referencing the PR for doc comments for Dart & Flutter as mentioned in the original issue here. Also, there were some comments missing for the client.ts.twig file (for RealtimeResponse, RealtimeRequest, RealtimeResponseEvent etc), I have added these from my end.

Please lmk in case I have missed anything, I'll add it asap.

@lohanidamodar
Copy link
Member

Hi @Im-Madhur-Gupta great work, we would like to proceed with this, if you are still interested to complete this, can you please sync this with latest changes. No worries if you are no longer working on this just let us know. Thank you.

@Im-Madhur-Gupta
Copy link
Contributor Author

Im-Madhur-Gupta commented Mar 17, 2024

Sure @lohanidamodar, let me sync my branch with master. Will do this in a bit.

@Im-Madhur-Gupta
Copy link
Contributor Author

@lohanidamodar I have merged my branch with master, pls check. Thanks.

@gewenyu99
Copy link

Hey there! There were a lot of big PRs during this Hacktoberfest, and we wanted to give everyone ample time to collaborate with our engineering team. If you were able to merge your PRs during October, amazing. If it’s still not merged, don’t worry about it either. Either way, we’ve got your Hacktoberfest swag minted and ready to ship.

Please comment with your Discord username here so we can contact you about your shipping information to deliver your Hacktoberfest swag.

@Im-Madhur-Gupta
Copy link
Contributor Author

Hi @gewenyu99, thank you, here is my Discord username: immadhurgupta

@@ -1,7 +1,7 @@
# {{ spec.title }} {{sdk.name}} SDK

![License](https://img.shields.io/github/license/{{ sdk.gitUserName|url_encode }}/{{ sdk.gitRepoName|url_encode }}.svg?style=flat-square)
![Version](https://img.shields.io/badge/api%20version-{{ spec.version|url_encode }}-blue.svg?style=flat-square)
![Version](https://img.shields.io/badge/api%20version-{{ spec.version | split('.') | slice(0,2) | join('.') ~ '.x' | url_encode }}-blue.svg?style=flat-square)
Copy link
Member

Choose a reason for hiding this comment

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

@Im-Madhur-Gupta Where did this change come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, facing some medical issues currently, will get back to you in a couple of days.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lohanidamodar Apologies for the delay in reply, I added this code while I was working on the ticket to fix some issue with the version batch but as I can see now we don't need it, hence I have reverted this change and pushed it.

@lohanidamodar lohanidamodar merged commit 264aca0 into appwrite:master Jun 4, 2024
33 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants