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

Added project for code review #1

Merged
merged 1 commit into from
Nov 1, 2021
Merged

Conversation

thecodechaser
Copy link
Owner

This branch includes the following features:
-Copy project from trails for code-review
-Added linter config files

-copy project from trails to review
a
-add linter config files
Copy link
Owner Author

@thecodechaser thecodechaser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @thecodechaser,

Good job so far! Your project is already looking nice

There are some issues that you still need to work on to go to the next project but you are almost there.

To Highlight

  • Your PR is on point 👍.
  • There are no linter errors.
  • Descriptive Readme file.
  • Clean code.

Required Changes ♻️

Check the inline comments under review

Optional suggestions

  • N/A

Cheers and Happy coding! 👏 👏 👏

Comment on lines +12 to +16
<nav class="sticky">
<div class="main-choose"><a href="#welcome-section">About</a></div>
<div class="main-choose"><a href="#projects">Work</a></div>
<div class="main-choose"><a href="#footer">Contact</a></div>
</nav>
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Please use list <ul> or <ol> for the social links instead of <div>. Whenever we have a list of similar or identical elements, we should group them together on a list. It is also better for SEOs interpretation. 😉

Comment on lines +71 to +75
<div id="footer">
<p class="footer-content">**This is just a fake portfolio. All the projects and contact details given are not
real.</p>
<p class="footer-content" style="margin-left: 200px;">© Created for Microverse</p>
</div>
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Please use the <footer> tag for the footer content instead of <div>. We should always use semantic tags as we have gone through the lesson it's extremely important. Can you do this for us? 😄

Comment on lines +63 to +69
<div id="links">
<a class="social-media" href="https://www.facebook.com">FaceBook</a>
<a class="social-media" href="https://www.github.com">GitHub</a>
<a class="social-media" href="https://www.twitter.com">Twitter</a>
<a class="social-media" href="email@gmail.com">Send an email</a>
<a class="social-media" href="+2123456789">Call me</a>
</div>
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • As I mentioned above, Please use list <ul> or <ol> for the social links, I think you can put these links as a list of items. Whenever we have a list of similar or identical elements, we should group them together on a list. It is also better for SEOs interpretation. 😉

Copy link

@bimbolabuari bimbolabuari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Project Approved 🚀

Hi @thecodechaser 🥇,

Your project is complete! There is nothing else to say other than... it's time to merge it :shipit:
Congratulations! 🎉

Highlights

  • You left the comment under the block of code that fits the context of the comment 👍
  • You phrased the comments politely, as suggestions instead of order ✊
  • You include an explanation, a "why", for each requested change 🎉
  • Your comments are clear and easy to understand ✔️

Optional suggestions

Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.

  • While ol tags get you ordered lists, ul tags get you bullet points, it is more preferred to use ul to list unordered items.

Cheers and Happy coding!👏👏👏

Feel free to leave any questions or comments in the PR thread if something is not 100% clear.


As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.

@thecodechaser thecodechaser merged commit b3a50b7 into main Nov 1, 2021
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.

2 participants