Skip to content
This repository has been archived by the owner on Jan 12, 2019. It is now read-only.

Convert project to ES6 / add ESLint #134

Open
mbifulco opened this issue Feb 18, 2017 · 3 comments
Open

Convert project to ES6 / add ESLint #134

mbifulco opened this issue Feb 18, 2017 · 3 comments

Comments

@mbifulco
Copy link

mbifulco commented Feb 18, 2017

Hi there -
I'm interested in contributing more to this project, but for a variety of reasons, it would be easier to do so if the repo were written in ES6 Syntax. I don't know how that fits into the overall plan for this repo/project/firebase, so I figured I'd ask if you'd accept such a PR before I go and do the work.

Ultimately I'm thinking of going in this direction:

  • converting this project to es6 using babel
  • adding ESLint to the project
  • updating the repo to be used as an importable NPM package that can be fired up with a config (much like the node firebase and firebase-admin libraries already are).

Would love to hear what you think!

@katowulf
Copy link
Contributor

Why would one need to use babel, considering that es6 is already supported in Node.js? Seems like we could just start using it. Linting sounds fine.

Not sure what the point of making the lib importable is, but would love to hear some justifications for that and see a usage sample.

@mbifulco
Copy link
Author

You know, I just straight up didn't realize es6 was supported in Node. Sorry about that - I do still think it's a good idea to convert to es6 syntax, though.

The case for making the library importable seems obvious to me - Until firebase has better querying functionality and/or easier search, I need to run flashlight to keep an elasticsearch index up to date. Flashlight should run as a don't-think-about-it service that I feed a config.

As flashlight gets smarter, I shouldn't have to think about the technical details of updating the flashlight-part-of-my-project (wherever it runs). In my case, flashlight runs in a node app alongside some other serverside jobs that help service a product that is growing in complexity. I'd like to maintain them all in one place, as a part of a single repo.

Does that make sense? It's possible my thinking is flawed (which has happened, from time to time).

@mbifulco mbifulco changed the title Convert project to ES6 with babel / add ESLint Convert project to ES6 / add ESLint Feb 24, 2017
@katowulf
Copy link
Contributor

For linting and es6 conversions, some small PRs with accompanying test units would be fine. I wouldn't support any large scale code changes mostly because we don't have sufficient tests to ensure we won't introduce significant bugs.

The modularity makes sense in theory, but I'm not really seeing the point in practice yet. Again, some real use cases may help here. Taking a look at the app.js file, it feels like there are some nuances to getting ES up and runnable that won't jive very well with making it more of an importable service.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants