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

exe/jekyll comments #9335

Closed
wants to merge 2 commits into from
Closed

Conversation

danielhaim1
Copy link
Contributor

Summary

Added comments to the code to explain its purpose and functionality.

This is a 🔦 documentation change

@danielhaim1 danielhaim1 changed the title exec/jekyll documentation exe/jekyll comments Apr 6, 2023
@ashmaroli
Copy link
Member

Hello @danielhaim1, while I agree that comments to code help improve comprehension, I also believe that too much of something does more harm than good.

Also, your commentary for $LOADPATH.unshift is wrong. The instruction doesn't mean to "force printing something.." at all. Perhaps you meant to place it elsewhere? 😀

On the whole, I am not with the proposal / idea. Deferring for a second-opinion..

Pinging @parkr.

@ashmaroli ashmaroli requested a review from parkr April 6, 2023 17:16
@danielhaim1
Copy link
Contributor Author

danielhaim1 commented Apr 6, 2023

@ashmaroli Hold up, let me check the code. I had some trouble with branching this morning and things got really messy. Let's close this pull request, I'll review and amend in another one.

@danielhaim1
Copy link
Contributor Author

@ashmaroli fixed, I had two branches for this file, but the comment was missing in one of them due to syncing issues. I'm currently reviewing and documenting the code to gain a better understanding to contribute more effectively.

@legionerlegion
Copy link

legionerlegion commented Feb 11, 2024 via email

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

3 participants