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

Initial setup #1

Merged
merged 57 commits into from
Jun 24, 2019
Merged

Initial setup #1

merged 57 commits into from
Jun 24, 2019

Conversation

medikoo
Copy link
Contributor

@medikoo medikoo commented Jun 11, 2019

Cross project ESLint & Prettier config

Initial version that reflects rules we rely on in serverless project, with following improvements:

  • Stripped airbnb base (we relied on old version, not Node.js friendly which enforced browser specific rules). Instead our base is eslint:recommended
  • Configured to not interfere with Prettier (all colliding rules were turned off)
  • Based on latest version of ESLint
  • Two dedicated entry points:
    • node.js - Config for Node.js projects
    • browser.js - Config for Browser projects
  • Open for further changes and improvements

Commitlint config

Currently experimental (practiced only in context of this repository), an attempt to rely on Convention Commits Convention, which will allow us to auto generate and improved CHANGELOG.md files.

See Commitlint and automation of release process for more info

Meta configuration for this repository

  • EditorConfig
  • package.json
  • ESLint integration
  • Prettier integration
  • Commitlint integration
  • Travis CI configuration
  • License
  • Documentation

@medikoo medikoo self-assigned this Jun 11, 2019
@medikoo medikoo force-pushed the initial-setup branch 2 times, most recently from 7c93448 to ced2386 Compare June 11, 2019 12:45
@medikoo medikoo force-pushed the initial-setup branch 2 times, most recently from e78c1f8 to 2c2a9bd Compare June 19, 2019 14:54
@medikoo medikoo requested a review from pmuens June 21, 2019 07:42
Copy link

@pmuens pmuens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 👍

It's great to have one source of truth for this kind of config 👌 Just added some minor comments.

LICENSE Outdated
@@ -0,0 +1,15 @@
ISC License
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we have a company-wide policy we follow here but usually new stuff we do uses Apache-2.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I confirm with @ganeshvlrk on that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ganeshvlrk suggested we use MIT, to be consistent with other projects.

I've switched it to MIT then

module.exports = {
extends: 'eslint:recommended',
env: { es6: true },
parserOptions: { ecmaVersion: 2015 },
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this specific version and not something newer?

Copy link
Contributor Author

@medikoo medikoo Jun 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It reflects Node.js v6 which we currently support in serverless project. Anything greater will allow syntax that may crash v6.

We may put greater version here, but then in serverless we should override it.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that sounds reasonable. I was just asking because I'm not sure which projects use which Node.js version and maybe there's a tradeoff to be made here that we use newer version here and use v6 stuff only for the Framework (if that's the only project holding us back).

Hopefully we can get rid of Node.js 6 support soon.

package.json Outdated Show resolved Hide resolved
@medikoo medikoo requested a review from pmuens June 24, 2019 07:18
Copy link

@pmuens pmuens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :shipit:

We can keep Node 6 support for the time being and update this config later on.

@medikoo medikoo merged commit 249c471 into master Jun 24, 2019
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