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

Review Past Project Survey Form #2

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Review Past Project Survey Form #2

wants to merge 9 commits into from

Conversation

NahnahAJ
Copy link
Owner

@NahnahAJ NahnahAJ commented Jan 5, 2023

Project Requirements

This PR adds the following changes to the Survey Form

  • Create a new empty repo.
  • Copied my code from the project that was completed during the Admissions Trial
  • Built a survey form using HTML and CSS
  • Followed FCC's guidelines
  • Made sure that tests and requirements were met
  • Create a new branch for my copied code.
  • Open a pull request with the changes.

Copy link
Owner Author

@NahnahAJ NahnahAJ left a comment

Choose a reason for hiding this comment

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

Hi @NahnahAJ ,

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!

Highlights

🏁 Linters are passing
🏁 Meaningful commit messages
🏁 Good PR

Required Changes ♻️

Check the comments under the review.

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.

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.

Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.


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.

Comment on lines +1 to +3
body {
background-image: url("../img/bg.jpeg");
background-size: cover;
Copy link
Owner Author

Choose a reason for hiding this comment

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

The background image here is broken. Fix it.

Copy link

@wayungi wayungi Jan 5, 2023

Choose a reason for hiding this comment

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

Good work pointing out the broken background image.

  • Please always be polite and kind to those whose code you review by requesting changes and not ordering for them. Always use polite terms like please, kindly,... 👀

  • It is also a good practice to explain why a change should be made. This helps the student to make the change and also learn in the process. 🧑‍🏫

  • Whenever possible, please offer examples or links to resources that may help the student find solutions to the problem 👍🏾

Comment on lines +87 to +89
</div>

<button id="submit">Submit</button>
Copy link
Owner Author

Choose a reason for hiding this comment

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

Make sure to remove spaces from your code

Copy link

@wayungi wayungi Jan 5, 2023

Choose a reason for hiding this comment

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

Nice job looking through this code base and picking out this violation.

  • You did well using inline comments and pointing out exactly where the change should be made. This makes it easy for the student to make the change. 👏🏾

  • As requested above, please always be kind and polite in your choice of words. Always request changes politely. 👍🏾

  • Please always remember to tell a student why a change is needed. This helps them learn in the process. 🧑‍🏫

Comment on lines +81 to +84
</div>
</div>

<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.

Make sure to remove spaces from your code

Copy link

Choose a reason for hiding this comment

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

Thank you for pointing out the many blank spaces in this project. The student will surely learn not to leave non-semantic blank spaces in the code.

  • As a way to keep the student motivated and eager 😀 to make that changes, always recommend them for the good work done and politely request for the changes to be made.

  • Code reviews also teach 👨🏾‍🏫 students good coding practice so please always remember to state why a change is required.

  • Consider including a question or two to encourage the person to ask for more help or clarification if needed. This can make the review process more collaborative and interactive. 🤝

Comment on lines +45 to +55
<div class="input-group">
<p>Would you recommend Havier Medical Service to a friend?</p>
<div>
<input type="radio" name="recommend" value="1" >Definitely
</div>
<div>
<input type="radio" name="recommend" value="1">Maybe
</div>
<div>
<input type="radio" name="recommend" value="1">Not Sure
</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.

Same as before check my comment

Copy link

@wayungi wayungi Jan 5, 2023

Choose a reason for hiding this comment

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

Thank you for stressing the fact that non-semantic spaces in code are not good. 👍🏾

  • Please consider offering the student a link to a resource where they can learn the cons of non-semantic spaces in code 👩🏾‍🏫
  • Always be polite in your change requests. This encourages friendly communication between the student and the reviewer 🤝

Comment on lines +20 to +31
<input type="text" id="name" placeholder="Enter your name">
</label>
</div>
<div>

<label id="email-label">Email
<input type="email" id="email" placeholder="Enter your email">
</label>
</div>
<div>
<label id="number-label">Age
<input type="number" id="number" placeholder="Choose Age" min="18" max="60">
Copy link
Owner Author

Choose a reason for hiding this comment

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

There was no required keyword at the end of input tags fix it.

Copy link

Choose a reason for hiding this comment

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

Good work using inline comments to point out exactly where the change should be made.

  • Please always explain why a change is required to the student to give the student some context. 😀
  • You can also let the student know that you are available to offer help and encourage them to reach out in case they do not understand the change request. 👍🏾
  • Whenever needed, please share links to help the student better understand why certain features are required 🟢

Copy link

@wayungi wayungi left a comment

Choose a reason for hiding this comment

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

Hi @NahnahAJ ,

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

Below are some highlights and required changes that you should focus on:

Highlights

  • Uses GitHub code review tools well 💯
  • Uses inline comments 👍🏾
  • Able to identify bad practices in code 😃

Required Changes ♻️

  • Instead of giving orders, make suggestions. forexample "I think it would be better to use semantic tags" sounds nicer than "Use semantic tags," doesn't it?
  • Remember to explain why you request a change - it might be obvious for you but not for someone else.
  • Remember to be polite and clear in your comments - your job here is to help another person improve in a friendly way.
  • [OPTIONAL] You can include emojis and gifs to make your code review more human-friendly alien.

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.

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.

Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.


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.

2 participants