fac28 / code-reviews

0 stars 0 forks source link

[Space pets (Shaughn and Carlos] - code review #8

Open nichgalzin opened 1 year ago

nichgalzin commented 1 year ago

README

[Does the README describe the project using the subheadings: Why?, What?, How? i.e. Why have you created this repo, what does it do, how does it do it?]

 Needs a README file.

User stories

[Does the project meet the user stories for that week?]

 The site has a clear story user story and layout. Layout flows as you expect it to and sections are laid out neatly.  
 Ticks all the boxes for the core stories list.

Learning outcomes

[Does it demonstrate the learning outcomes for that week?]

 The it does a good job of laying out html semantically, Maybe things a overly nested sometimes. Since you asked Matt about the article tag Carlos, also not sure if the one used in your index page is necessary?  
Could make use of CSS primitives. But CSS is logically organized.

UI bugs

[Can you see any obvious bugs or areas to improve

  Sites mobile version could be improved a bit, text and some images on smaller screens can be hard to read and need padding and margins. All the links and form work well and logically and show you where you are in the navigation tree.

Instructions

[Does everything work as expected or were there missing instructions?]

All makes sense.

File structure

[When you open the project in your editor, does the file structure make sense?]

Everything is were you expect it and clearly labelled.

Flow of control

[ Can you you follow the different paths the code might take?]

Yep. Everything seems pretty logical.

Naming

[Do variables and functions have clear and descriptive names?]

Yes naming convention is logical and consistent. I like the use of  double underscores.

Readability

[Do you understand the code?]

Yes everything makes sense. You could have a stretch goal and try and use more js if you wanted to try and make the site more interactive.