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

Suggested tidy up in files, various odd hangovers #29

Open
developess opened this issue Nov 28, 2018 · 0 comments
Open

Suggested tidy up in files, various odd hangovers #29

developess opened this issue Nov 28, 2018 · 0 comments

Comments

@developess
Copy link

I've been practicing this code-along in preparation for next week. There's a few odd things about it that I think could do with tidying up/removing.

Minor things:

  • There's a random data.sql file in walkthroughSteps > step-1 > src. It's never referenced in the notes, the only SQL file is db_build.sql. I think it should be removed as it's confusing.
  • the http node module is imported in every handler.js file and never used. Not sure why it's there. I think it should be removed in all 5 steps.
  • Theres a commented out line in step-1 handler.js:
// const getData = require('./dynamic.js');

The uncommented-out version appears in Step 2-4. However in the mentor notes we write this line during Step 5. I suggest we get rid of all references to this until it is brought in in step 5, as it is not relevant until then.

One slightly more major thing but still not that big a deal:

With those changes, the code in Step-1 is identical to that in Step-2. If you look at the walkthough mentor notes, Step 1 is just looking in each file and getting the project running locally. No changes to code occur.

I suggest we rename 'Step-1' in the mentor notes to 'Introduction and Setup' and Step-2 becomes Step-1 and so on so there are actually only 4 steps in the repo instead of 5.

I also suggest that the 'Getting Started' mentor notes be expanded upon. In addition to git cloning, you need to:

$ cd pg-walkthrough/walkthroughSteps/step-1
$ npm install

before npm start will work.

It may help a lesser-prepared mentor in future get started without tripping up.

I would like to address these suggestions and plan on making a PR before next week.

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

No branches or pull requests

1 participant