jmartinez95 / MI-449-SS17-741-js-server-intro-jtrO3q

0 stars 0 forks source link

Project Feedback #1

Open jmartinez95 opened 7 years ago

jmartinez95 commented 7 years ago

@stuartpearman Can you take a look at this? It's hosted here and meets the following criteria:

First try?????

egillespie commented 7 years ago

Fantastic work on this project! The JavaScript code looks great.

Alas, there are a few things I'd like you to update. And don't worry about not getting any of these on the first try. It's extremely rare to complete a business feature without having to tweak something.

</br> is not a valid HTML tag

We're not using a full HTML document, but we do want the HTML you do serve up to be legit. :wink:

In general, you should avoid using br except in the situations described in this lesson. </br> is also not syntactically correct so you're putting heavy faith in the browser gods to render it correctly and consistently.

I recommend just removing all of those br tags you're using.

Every page that is not the homepage (/) contains a link to it

I don't see a link to your homepage on /bad-page or any other bogus page, so the last criteria is not being met. Could you add this in?

Remove semicolons ;

While it does stink that linter isn't working in Atom at the moment, one easy bad habit to break (and fix) is the use of semicolons. When we introduced the JavaScript linter, half of the first page of the lesson was dedicated to explaining why.

Linters help cover more subtle style issues, but we still want students to be coding using modern practices, so we'll continue working with people who have been coding a while to help bring their habits into the current decade. :stuck_out_tongue_winking_eye:


After you’ve made your changes and pushed them to GitHub and your hosted site, give it a once-over to make sure it looks right, then comment back here and I’ll take another look.

Thanks! :rocket:

jmartinez95 commented 7 years ago

@egillespie Fixed

egillespie commented 7 years ago

Awesome, everything looks and works great!

Chris showed told me standard can be installed and used from the terminal to lint your JavaScript:

npm install --global standard
standard server.js

It's not as convenient as linting in Atom, but we do care about code quality so I recommend installing this and using it until Atom starts behaving. When working in a large team or open source project with code style guides they would expect no less and that's exactly what we're trying to prepare everyone for. 😉

Good work on the follow-up! :shipit: