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

Close div tag in resume.hbs #16

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ciderpunx
Copy link

Fixes: #15

@ishu3101
Copy link
Contributor

You should convert your indentation from tabs to spaces.

@ciderpunx
Copy link
Author

@ishu3101 - the rest of the code uses tabs, so I thought it better to be consistent.

@ishu3101
Copy link
Contributor

I see that in the commit that you have made above, the git diff is showing extra tabs have been added before each line (as shown in the screenshot below) which means the alignment of the code is not write and what git diff is telling is that you have made changes to line 16 - 27 instead of just adding the closing div tag.

image

@ciderpunx
Copy link
Author

ciderpunx commented Jan 23, 2017

Ah right, got you. I've de-indented so the indentation follows the original way of doing it.

close_div_tag_in_resume_hbs_by_ciderpunx_pull_request_16_jsonresume_jsonresume-theme-boilerplate_-_2017-01-23_15_09_11

@ishu3101
Copy link
Contributor

I noticed that you have also removed the indentation of the closing body tag as well.

to address jsonresume#16 (comment).
Also re-added empty line between opening body tag and opening tag of #resume div.
@ciderpunx
Copy link
Author

Re-added, also added back the empty line directly after the opening body tag.

@ishu3101
Copy link
Contributor

Great. You should squash your commits into a single commit as it makes no sense to pollute the history with 3 commits of you just fixing things. A single commit of everything correct is the correct way to go.

ciderpunx pushed a commit to ciderpunx/jsonresume-theme-boilerplate that referenced this pull request Jan 25, 2017
With indentation per discussion thread: jsonresume#16

Fixes: jsonresume#15
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