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

Sophmore page #21

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

Conversation

jay-deep
Copy link

@jay-deep jay-deep commented Oct 5, 2020

Bro , Now check it, is it fine?
Will definitely enhance the css but main thing is code, I tried so hard , You assigned me so literally I dont wanted to let u down, Plz inform me the errors in this.
Fixes #3

Bro , Now check it, is it fine?
Will definitely enhance the css but main thing is code, I tried so hard , You assigned me so literally I dont wanted to let u down, Plz inform me the errors in this.
JsrEntity#20
@jay-deep
Copy link
Author

jay-deep commented Oct 6, 2020

Bro can u add hactobefest in your label topic?

@decon-harsh
Copy link
Member

decon-harsh commented Oct 6, 2020

See @jay-deep This PR is quite good, Here are few things i would suggest..

  • Copy the navbar code from index.html

  • Link stylesheet styles.css to sophomore pages (don't write same css multiple times)

  • Copy whole head from index.html

  • Write custom css for some parts, tryna use bootstrap defined classes.

  • Avoid this much use of br tags.

  • Dont add css directly in html file , write css in a file sophomore.css and place it in css directory , link the stylesheet from there.

  • Dont open a new pr make changes in your local , push it your fork it will automatically reflect in this PR

@decon-harsh
Copy link
Member

I added these tags for now but you have to complete this, or else I am forced to close this. Thank you :)

@jay-deep
Copy link
Author

jay-deep commented Oct 6, 2020

Bro actually , right now I don't know how to use bootstrap
I have did some changes that u suggested

Linked new sophomore.css

added navbar code from index.html

Removed some br

and bro I pushed it to my fork

@decon-harsh
Copy link
Member

decon-harsh commented Oct 6, 2020

That's great, so do you want to enhance it's CSS or should I take it from here. Work both ways
Since you had completed hacktober, I would encourage you to take this up as a new thing to learn. It will increase your skills surely.

@jay-deep
Copy link
Author

jay-deep commented Oct 6, 2020

yeah definitely bro, it was never about hacktober fest, its just as like till 1st october , I didnt even knew how to use github, now atleast I am happy that I know a little bit. Bro Do 1 thing na, You Merge this in master and if I got something to enhance in it, I will do it parallely? What say?

@decon-harsh
Copy link
Member

yeah definitely bro, it was never about hacktober fest, its just as like till 1st october , I didnt even knew how to use github, now atleast I am happy that I know a little bit. Bro Do 1 thing na, You Merge this in master and if I got something to enhance in it, I will do it parallely? What say?

This will mess the commit logs , multiple commits for same work is not appreciated. However if you want I can merge this is new branch and when done , will merge this in master will a single commit(Squashing) .

@jay-deep
Copy link
Author

jay-deep commented Oct 6, 2020

Yup that's also great

@decon-harsh
Copy link
Member

I checked once merged , i guess you can't edit the commits . Then you have to send multiple PRs that's not cool.

If a PR is not merged and you add new commit it will automatically show. Better to not merge it now.

@jay-deep
Copy link
Author

jay-deep commented Oct 6, 2020

But now, how will u add this sophomore page in master branch?

@decon-harsh
Copy link
Member

But now, how will u add this sophomore page in master branch?

I will wait till you complete this task ;)

@jay-deep
Copy link
Author

jay-deep commented Oct 6, 2020

Now I just need to enhance it na, I mean no need to add something na???

@decon-harsh
Copy link
Member

Now I just need to enhance it na, I mean no need to add something na???

Yes enhance the styles

Did some enhancement in css
@jay-deep
Copy link
Author

jay-deep commented Oct 6, 2020

Bro I did some enhancements , I don't think more enhancement is needed in this, Yes if there's something that u want to add then plz inform and bro when u merge it na plz put hacktoberfest-accepted label on it
Capture
Because I think according to new rules only the PR with these label are counted

@decon-harsh
Copy link
Member

Screenshot_2020-10-07-00-55-54-648_com.github.android.jpg

The labels are already there

@jay-deep
Copy link
Author

jay-deep commented Oct 7, 2020

okay cool, bro Did u checked the enhancements?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make another page /sophomore
2 participants