-
Notifications
You must be signed in to change notification settings - Fork 0
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
Test branch into main branch merge #5
Conversation
Coolczycki
commented
Mar 24, 2021
•
edited
Loading
edited
- Configure HTML structure for the project
- Configure CSS styling for the project
- Flexbox & grid setup
There was a problem hiding this 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 team 👋
It's impressive to see how well you've done considering it's your first project at Microverse 👍 👏
However, there are still some changes to be made before this milestone can be approved.
REQUIRED
-
[ DONE] Your commit messages should be capitalized, and written in the imperative tense. You can watch this video on how to edit your commit messages using
git rebase
-
Comment; Commit messages updated; they are now capitalized and written in the imperative tense.
Thank you for the material. -
[ DONE] Your README doesn't look professional. You may want to use this Microverse template
-
Comment; README file updated. Microverse template applied.
-
[ DONE] Your Pull Request should contain a brief description of what was accomplished (screenshot from Pathwright)
-
Comment; Description added for the pull request. The screenshot you sent me over contains a link I
cannot use it since it's a screenshot. -
[ ADDITIONAL INFO REQUIRED] Kindly resolve the conflicts in your Pull Request
-
Comment; Requested additional information from the reviewer.
-
[ DONE] The first image on your page is too big. Kindly reduce the size.
-
Comment; First image size reduced.
-
[ DONE] The main content section has a very small width. You should increase the width
-
Comment; Width increased by 10%
-
[ DONE]
Theory of Inflation
section is distorted. You should fix it. -
Comment; Section sorted out.
-
[ DONE] Reduce the font-size of elements in your footer and add a horizontal rule to separate the footer from the other section above it.
-
Comment: Horizontal rule added (hr line). The font size of elements in the footer reduced.
-
[ DONE] Elements aligned with a combination of all three techniques (float, flex and grid). (Screenshot from Pathwright) and (Screenshot from Odin). You didn't make use of
grid
-
Comment; Grid applied to Theory of Inflation section.
"As described in the Code reviews limit Policy you have 2 code reviews left for this project."
Happy Coding 💻
Feel free to reach out to me on slack @Elijah Ayandokun
Elijah O. Ayandokun
Github | LinkedIn | Twitter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Status: Changes requested 🔴
Hello guys 👋
Thank you for making all of the requested changes, but I still can't approve your project with merge conflicts and without the linters check. I believe linters are not being checked on this repository because you have conflicts with the base branch. Can you please fix the conflicts and check if it will work?
As described in the Code reviews limit Policy you have only 1 code review left for this project. If you think that the code review was not fair, you can request a second-opinion using this form.
Kindly make the requested changes and submit them for another review, please 🙏
I will be on Slack 24/7 in case you have any questions about this review 🤝
Murilo Roque Paiva da Silva
- Github: @muriloroque
- Linkedin: MuriloRoque