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

Adds lint-staged configuration #293

Merged
merged 19 commits into from
Nov 15, 2017
Merged

Adds lint-staged configuration #293

merged 19 commits into from
Nov 15, 2017

Conversation

jbalsas
Copy link
Contributor

@jbalsas jbalsas commented Nov 10, 2017

Hey @Robert-Frampton, please, take this PR with care 😂

This applies the hopefully-final guidelines. I've separated the linting bits in packages for an easier digestion. Things to have in mind:

  • I might've messed up in https://github.com/eduardolundgren/eslint-config-liferay for a lerna setup, so tests are failing on CI now... 😭
  • I've left ignored some errors because fixing them seemed risky or broke tests. Those could be considered as TODOs
  • There might be a bug in prettier that prevents it from properly formatting lines of around 81-82 chars. I've ignored them for now as well
  • Based on the guidelines, optional params should always have a default value (rather than the opt_ prefix). I've left this for the future as well since it requires quite some time to go through it.

@robframpton
Copy link

Hey @jbalsas

This looks good to me. Just waiting to merge since a high priority issue came up for the wedeploy team today when they updated to v2.14.0, I think we should probably merge and publish #298 first as a patch and then merge these changes.

@jbalsas
Copy link
Contributor Author

jbalsas commented Nov 15, 2017

Hey @Robert-Frampton! We should be able to merge to develop and still do patch releases for these kind of issues. That was the whole point about this new branch strategy.

Just to validate, I'm going to merge this in, and then proceed to cut a release from #298... YOLO!

@jbalsas jbalsas merged commit 4aaa677 into develop Nov 15, 2017
diegonvs added a commit to diegonvs/metal.js that referenced this pull request Nov 20, 2017
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.

2 participants