search

dosco / graphjin

GraphJin - Build NodeJS / GO APIs in 5 minutes not weeks
https://graphjin.com
Apache License 2.0
2.91k stars 177 forks source link

Code clarity update #492

Closed rkrishnasanka closed 2 months ago

rkrishnasanka commented 5 months ago

The code currently is hard to work with because of the lack of documentation, the crazy number of features pack in and usage of shorthand variables.

This PR tries to improve the situation for the following scenarios:

  1. Renaming all struct fields to not have shorthand
  2. Replacing redundant variables
  3. Ensuring that all type structs are PascalCase rather than camelCase since it messes up exports and makes the code hard to read since there is no difference in the instance names and the typedef. This also has the advantage of making sure that all teh structs are available outside the package scope
vercel[bot] commented 5 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
graphjin ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 12, 2024 0:14am
dosco commented 5 months ago

Sent you a couple thoughts on discord

dosco commented 5 months ago

3. Ensuring that all type structs are PascalCase rather than camelCase since it messes up exports and makes the code hard to read since there is no difference in the instance names and the typedef. This also has the advantage of making sure that all teh structs are available outside the package scope

let me know your thoughts on this since "not exposing" the structs outside the package scope was the point of keeping them camelCase. The idea is that those structs etc are part of the internal API and should not be exposed as they would unnecessarily need to be included in the backward compatibility guarantee slowing down development and also bloating the api documentation.

CLAassistant commented 2 months ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: rkrishnasanka
:x: deepsource-io[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

rkrishnasanka commented 2 months ago

@dosco I know this took a long time but I basically undid all the public structs and helped rename a lot of the variables to get a better idea about the flow.

If this is good, I'll restart the multischema support, we definitely need it on our end

dosco commented 2 months ago

Thats a huge PR did you manually add these comments or is there a tool that helps?