Skip to content

Work branch#1

Open
qusai122 wants to merge 2 commits intomainfrom
work-branch
Open

Work branch#1
qusai122 wants to merge 2 commits intomainfrom
work-branch

Conversation

@qusai122
Copy link
Owner

start my project

@qusai122
Copy link
Owner Author

qusai122 commented Feb 22, 2023

Good job so far! 👍
There are some issues that you still need to work on to go to the next project but you are almost there!

Required Changes ♻️

  • Kindly check the inline comments under the review. ⬇️

To Highlights

✔️ No linter error.
✔️ Proper Github flow used.

Optional suggestions

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

  • The commit message should be written in the imperative tense and the first letter should also be capitalized. Ex: " Creat index file " creat index file". For more details...

Cheers and Happy coding!👏👏👏

@qusai122
Copy link
Owner Author

qusai122 commented Feb 22, 2023

  • try to provide a good PR title & description.

@qusai122
Copy link
Owner Author

qusai122 commented Feb 22, 2023

  • describe your project in 1 or 2 sentences, and your read-me should follow the template.

Copy link

@didierganthier didierganthier left a comment

Choose a reason for hiding this comment

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

Hi @qusai122 ,

🎉 Good job so far! You're making great progress on this project, but there are still a few areas that need attention before we can move on to the next step.

Highlights

  • Added 2 comments ✔️
  • Showed highlights ✔️

Required Changes 🛠

  • Well done overall 🚀 but you should submit a review in the Pull Request utilizing the "Review changes" feature from GitHub. You can learn here

  • Nice work on the comments 🗨️ but you should use the GitHub inline comments feature at least once to leave review comments 😉

  • The 2 comments added are good 💯 but make sure to actually explain why you request the changes and make it sound like a suggestion instead of an order. 🙂

  • Please review the comments under the review for specific changes that still need to be made.

Optional Suggestions 💡

  • While these comments with the [OPTIONAL] prefix won't prevent the approval of this pull request, I strongly recommend taking them into account as they can improve your code.

🚀 Keep pushing forward, you're almost there! If you have any questions or need clarification on anything, feel free to leave a comment in the pull request thread and don't forget to tag me so I'll receive the notification.

Please note

  • To avoid confusion and delays, please do not open a new pull request for re-reviews. Use the same pull request submitted for the first review, unless otherwise instructed.

As per the Code reviews limits policy, you have a limited number of reviews per project. If you feel that the code review was unfair, you can request a second opinion using this form.

Cheers and happy coding! 👏👏👏

qusai122

This comment was marked as duplicate.

Copy link
Owner Author

@qusai122 qusai122 left a comment

Choose a reason for hiding this comment

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

Good job so far! 👍
There are some issues that you still need to work on to go to the next project but you are almost there!

Required Changes ♻️
Kindly check the inline comments under the review. ⬇️
To Highlights
✔️ No linter error.
✔️ Proper Github flow used.

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

The commit message should be written in the imperative tense and the first letter should also be capitalized. Ex: " Creat index file " creat index file". For more details...
Cheers and Happy coding!👏👏👏

@@ -0,0 +1,148 @@

Copy link
Owner Author

Choose a reason for hiding this comment

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

  • we can't see the readme file, kindly Fix that.

<section id="hero">
<h2>Handcrafted,home-made masterpieces</h2>
<form id="form" action="https://www.freecodecamp.com/email-submit">
<input
Copy link
Owner Author

Choose a reason for hiding this comment

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

We asked for username input also, but I think you forget about it.

width: 60vw;
}

@media (max-width: 650px) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

max-width should be 750 px

Copy link

@brainconnect93 brainconnect93 left a comment

Choose a reason for hiding this comment

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

STATUS: Changes Required ♻️

Hi @qusai122, 👋

Good job so far!
There are some issues that you still need to work on to go to the next project but you are almost there!

To Highlight 🥇

  • Has a proper Pull Request. ✔️
  • No Linter errors ✔️
  • Good use of Github flow ✔️

Changes Required

  • A requested by the previous reviewer, kindly give reason WHY you are requested the changes and should be inform of a suggestion rather than been an order

    • N.B:
      1. The student always includes an explanation, a "why", for each requested change. For example: This is a well-explained request: Why don't you use an article tag for the .first-article element instead of a div, that would improve the HTML structure and make the page more accessible.
      2. The student phrased the comments politely, as suggestions instead of orders. For example: This is a polite suggestion: *"I think it would be better to use semantic tags"
        This reads as an order: *"Use semantic tags"

This request lacks a "why": Why don't you use an article tag here instead of a div?

  • Please check the comments under the review.

Optional suggestions

Every comment with the [OPTIONAL] prefix won't stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better. Some of them were simply missed by the previous reviewer and addressing them will really improve your application.

Cheers and Happy coding!👏👏👏

Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me on( @brainconnect93 ) in your question so I can receive the notification.


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.

Copy link
Owner Author

@qusai122 qusai122 left a comment

Choose a reason for hiding this comment

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

Good job so far! 👍
There are some issues that you still need to work on to go to the next project but you are almost there!

Required Changes ♻️

  • Kindly check the inline comments under the review. ⬇️
  • We can't see your readme file, probably because you merged it.

To Highlights

✔️ No linter error.
✔️ Proper Github flow used.

Optional suggestions

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

  • The commit message should be written in the imperative tense and the first letter should also be capitalized. Ex: " Creat index file " creat index file". For more details...

Cheers and Happy coding!👏👏👏

<meta name="viewport" content="width=device-width, initial-scale=1.0">
<link href="https://cdn.jsdelivr.net/npm/bootstrap@5.3.0-alpha1/dist/css/bootstrap.min.css" rel="stylesheet">
<title>Document</title>
<link rel="stylesheet" href="style.css" />
Copy link
Owner Author

@qusai122 qusai122 Mar 1, 2023

Choose a reason for hiding this comment

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

  • kindly on this line of code! please include the given CSS through the stylesheet file because it will make your code professional and clear to anyone!

<section id="hero">
<h2>Handcrafted,home-made masterpieces</h2>
<form id="form" action="https://www.freecodecamp.com/email-submit">
<input
Copy link
Owner Author

@qusai122 qusai122 Mar 1, 2023

Choose a reason for hiding this comment

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

  • kindly add a user name input because it is a requirement.

width: 60vw;
}

@media (max-width: 650px) {
Copy link
Owner Author

@qusai122 qusai122 Mar 1, 2023

Choose a reason for hiding this comment

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

  • kindly fix the max width it's better to be 750px to match the project,

because some phones have a bigger width.

Cheers and Happy coding!👏👏👏

Copy link

@binhussen binhussen left a comment

Choose a reason for hiding this comment

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

Hi @qusai122 ,

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

Highlights

  • All linter checks passed ✔️
  • Descriptive pull request ✔️
  • Used GitHub flow ✔️

Optional suggestions

Every comment with the [OPTIONAL] prefix won't stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better. Some of them were simply missed by the previous reviewer and addressing them will really improve your application.

Cheers and Happy coding!👏👏👏

Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.


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.

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.

4 participants