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

Fix all eslint errors in the application #188

Closed
nickytonline opened this issue Sep 14, 2020 · 4 comments
Closed

Fix all eslint errors in the application #188

nickytonline opened this issue Sep 14, 2020 · 4 comments
Labels
good first issue Good for newcomers

Comments

@nickytonline
Copy link
Contributor

Goal

Now that we have the configuration being picked up thanks to #185, we can start fixing eslint issues. This is an all encompassing issues for eslint errors.

Reason

When you run eslint, there are over 200 lint errors.

Tips

  • First step is to run node_modules/.bin/eslint ./src/**/*.js --fix or use npx eslint ./src/**/*.js --fix to automatically fix any errors that eslint is capable of fixing.
  • It probably makes sense to have a PR that fixes the eslint errors for each page instead of one massive PR for fixing all the errors. It will be easier to grok and review.

Definition of Done

There are no more eslint errors and the application functions just as it did before the lint errors were fixed.

@nickytonline nickytonline added the good first issue Good for newcomers label Sep 14, 2020
@nickytonline
Copy link
Contributor Author

nickytonline commented Sep 14, 2020

Now that the eslint configuration is being picked up properly, there are errors in regards to missing prop types. You will need to run npm install prop-types to have access to prop types as they do not ship out of the box with Create React App (CRA).

One of the props is a job entity. I created the common proptype snippet for a job below. Add it to the codebase and import it wherever a job entity prop type exists.

import PropTypes from ‘prop-types’

const jobPropType = PropTypes.shape({
  id: PropTypes.number.isRequired,
  companyName: PropTypes.string.isRequired,
  companyLogo: PropTypes.string.isRequired,
  jobTitle: PropTypes.string.isRequired,
  roleFocus: PropTypes.oneOf([‘Front-end’, ‘Back-end’, ‘Full-stack’])
    .isRequired,
  postedAt: PropTypes.shape({
    nanoseconds: PropTypes.number.isRequired,
    seconds: PropTypes.number.isRequired,
    toDate: PropTypes.func.isRequired,
  }).isRequired,
})

export default jobPropType

The postedAt property is a FireStore TimeStamp object. It could also use the PropTypes instanceOf](),
e.g. PropTypes.instanceOf(Timestamp).

@drewclem
Copy link
Collaborator

Solid issue @nickytonline. I'll definitely chip away at this one as I can.

Our husky pre-commit hooks will fail any commit while these errors exist. Is there a way to disable that part of the process while these are getting resolved? From the command line? VScode git integration?

@nickytonline
Copy link
Contributor Author

nickytonline commented Sep 14, 2020

@drewclem, while we're fixing eslint issues, you can bypass husky via the --no-verify argument.

When committing code, run e.g. git commit "my awesome message" --no-verify

@drewclem
Copy link
Collaborator

Closed with #194 #195 #196

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants