Project Feedback

[zhezixi]

Introduce tools to an old web-based game

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

• [x] Start your project with these old game files
• [x] Convert the game's CSS to Sass, using variables, functions, and nesting where appropriate
• [x] Write a dev script in package.json that auto-compiles Sass and refreshes the browser at the same time
• [x] Write a build script in package.json that lints HTML, Sass, and JavaScript files before compiling Sass files (you can use these linters: htmlhint for HTML, stylelint and stylelint-config-standard for Sass, standard for JavaScript)
• [x] All linting problems are fixed (problems will be displayed in the terminal when you run yarn build)
• [x] Write a deploy script in package.json that builds the project and uploads it to Surge if there are no errors
• [x] Unnecessary files like node_modules and your compiled CSS do not exist in your repository
• [x] Unnecessary files like package.json and yarn.lock are not uploaded to Surge

[egillespie]

Hi @zhezixi, this project looks awesome! 😻 The Sass and ignore files look good, the game works, and your package.json is off to a great start.

Here are some recommendations for improvement:

Make stylelint a dev dependency

It looks like stylelint was added as a regular dependency instead of as a dev dependency:

  "dependencies": {
    "stylelint": "^9.2.0"
  }

The dependencies section of package.json is for tools your website uses while it is running. The devDependencies section, however, is for tools you use before the website starts.

Would you mind moving the line "stylelint": "^9.2.0" into the devDependencies section and then remove the entire dependencies section?

2. Call linters at start of dev and deploy scripts

Your linter scripts look great! Would you mind calling them at the start of your dev and deploy scripts so that any code quality issues can be identified while you are developing the website?

Right now, your linters work, but they don't run before you start your live server or deploy your changes to Surge, so there's a possibility that code issues may not be identified.

---

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! 🤘

[zhezixi]

dependency problem and linter issues fixed!

[egillespie]

Hi @zhezixi, this is a good step forward. I have a couple of suggestions to get your project the rest of the way:

1. Call build in your dev and deploy scripts

I see you added a build script that lints and compiles. This is awesome! ✊ The script isn't called as part of your dev script, though, and you're calling build:* in your deploy script, which is leaving out the lint:* scripts.

Could you do these two things?

1. Call build before dev:* in your dev script
2. Replace build:* with build in your deploy script

2. Add htmlhint as a dev dependency

If I run your HTML linter script, it isn't working because htmlhint is not listed in your devDependencies section. Would you mind adding this dependency so I can install and run the linter?

---

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! 🎸

[zhezixi]

I fixed the missing part and the website works fine. btw the only problem I have now is I cannot run yarn on my local terminal now. It always says there is an unexpected token in the end. I check the code but did not notice where I did wrong. Can you have a look and give me some advice?

[egillespie]

Hi @zhezixi, it looks like you have an extra , at the end of line 28. If you remove that, you should be able to run yarn again.

Another way you can identify those typos is to install the linter-jsonlint package in Atom. 😄

[zhezixi]

fixed!

[egillespie]

Sweet, this turned out really nice! 🤘

I don't have any other recommendations for improvement. Good work! :shipit: