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

Feature/navbar #17

Merged
merged 10 commits into from
Feb 26, 2018
Merged

Feature/navbar #17

merged 10 commits into from
Feb 26, 2018

Conversation

jenovs
Copy link
Contributor

@jenovs jenovs commented Feb 26, 2018

No description provided.

@nikrb
Copy link
Contributor

nikrb commented Feb 26, 2018

@jenovs great job Viktor.
Just a small point, looks like prop-types is missing from dependencies.
Are there any opportunities to add a test or two?

@jenovs
Copy link
Contributor Author

jenovs commented Feb 26, 2018

Added prop-types to dependencies and couple tests.

@nikrb
Copy link
Contributor

nikrb commented Feb 26, 2018

@jenovs hey viktor, sorry to be a pain, it looks great, but when I run npm install I get the following:

npm WARN ajv-keywords@3.1.0 requires a peer of ajv@^6.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN eslint-config-react-app@2.1.0 requires a peer of eslint-plugin-jsx-a11y@^5.1.1 but none is installed. You must install peer dependencies yourself.

Shouldn't these be in package.json or is this expected? Do I just install them locally?
When I run npm test:
Error: Cannot find module '/home/nik/projects/voyage4/bears-25/frontend/node_modules/jest-cli'
Is this missing or should I have it installed globally perhaps?

@jenovs
Copy link
Contributor Author

jenovs commented Feb 26, 2018

Regarding the first one: webpack/schema-utils#23 (looks like something we can ignore for now).
Regarding the second one: Incorrect version in package.json (should be v5: https://github.com/jenovs/cra-with-lintin/blob/master/package.json#L20)
I stand corrected. They updated dependencies. Looking into.
Deleted node_modules and lock file and reinstalled without warnings 🤔

all-good

Also I don't install anything globally (except node and npm).

@nikrb nikrb merged commit b43792b into develop Feb 26, 2018
@nikrb nikrb deleted the feature/navbar branch February 26, 2018 18:59
keith1111 pushed a commit that referenced this pull request Mar 25, 2018
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

Successfully merging this pull request may close these issues.

None yet

2 participants