-
Notifications
You must be signed in to change notification settings - Fork 42
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 ability to create gsoc page from data.yaml file #111
base: master
Are you sure you want to change the base?
Conversation
536815e
to
b95d3a4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! Good work on this. Would it better to replace variable names like c
, r
,q
,c1
,afb
,agh
etc with like proper words? That may increase the readability of the code.
Like c4
is probably column4
? afb
with probably facebook-link
and so on ?
@abhigyank I would rather replace c4 with row4 because it denotes the number of rows with four elements |
Sounds good. I couldn't understand what c4 was, so I guessed it would be column4. It would therefore definitely help to have well worded variable names. If it denotes "number of rows with four elements" why not name the variable as |
@abhigyank I have changed variable names and added comments. Let me know if the code is understandable now? |
Much better. Leaving it to @vsvipul to review :) |
Heads up @pranshukharkwal, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
Solves #110
@vsvipul