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

TransformerGenerator class extensive commenting #141

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alexaibio
Copy link

I am investigating your code in details and along the way made a lot of in-code commenting. Might be really useful for others studying this code, so I decided to PR.
I can continue if you like it

@subercui
Copy link
Member

Hi @alexaibio, thank you so much for the efforts! Upon reflection, I propose an alternative approach: what if we include a hyperlink in our project's README directing users to your annotated branch, rather than proceeding with a pull request? The reason is mainly that commenting styles can vary based on different personal preferences, and there are best practices advocating for brevity in commenting such as this. I acknowledge the importance of ensuring that our code is as clear and self-explanatory as possible. We will try to do so while keeping the comments concise and following best practices. Looking forward to your thoughts on this.

@alexaibio
Copy link
Author

alexaibio commented Jan 5, 2024

Hi @subercui ,
Thank you for your prompt response. Since it's your code, it is totally fine not to incorporate my comments, and there's no necessity to include a link to my branch.
I appreciate the principle of commenting brevity. I just thought that offering additional links to the article content could improve the learning curve and is in fact a providing external links to the article content.

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.

None yet

2 participants