evanshortiss/lintspaces-cli

Move cli into core

Opened this issue · 8 comments

schorfES commented

Hi @evanshortiss,

I was thinking about to move the cli support into the lintspaces module itself and bring it finally to a major release of 1.0.0. So we can keep new options and features in sync with the cli. I would like to use your knowlege to bring this feature into the library. What do you think about this approach?

Regards, Norman

evanshortiss commented
schorfES commented

Hi @evanshortiss,

I'm glad to hear that you want to join. First of all I will give you access to the repo. Then I think we should answer some questions.

  • Which minimal node version is required?
  • Should we update the core code to ES6, if required?
  • Should we update the testing and validation setup? (nodeunit vs. jest? jshint+jscs vs. eslint?)
  • How should the project file structure look like?
  • Do we need a build/publish process?
  • Do we want to be fully backwards compatible with options of lintspaces-cli? Which options to add?

These are the first questions and steps that came into my mind. What do you think?

Would we create a new issue and milestone for this in lintspaces?

Best, Norman

evanshortiss commented

@schorfES some responses inline below:

Which minimal node version is required?

I'd usually go with oldest LTS unless there's a reason not to. That'd be 6.x right now.

Should we update the core code to ES6, if required?

The CLI probably uses var instead of const, and could be updated to use forEach if we want. It's not something I'd worry about unless it makes life easier for users/maintainers. I'm not a fan of adding Babel unless it really gives value e.g async/await. For the CLI I don't think there's much need, but maybe for core ES6 could help?

Should we update the testing and validation setup? (nodeunit vs. jest? jshint+jscs vs. eslint?)

The CLI currently has no tests so that would be good. I like Jest, but don't mind using something else. I've started using "prettier" recently for code style. It allows me to just write code and then auto format on commit using "husky". It can be integrated with eslint if you need more specific linting.

How should the project file structure look like?

Looking at your current structure it seems pretty typical. Usually I'd add the CLI files in bin/ and keep the module core in lib/ like you currently have for "node-lintspaces". Did you have something else in mind?

Do we need a build/publish process?

Looks like you have a Travis build process in place. Were you thinking of adding an automated publish on PR merge with something like semantic-release? I've never used it, but could be nice.

Do we want to be fully backwards compatible with options of lintspaces-cli? Which options to add?

I'm not actively using lintspaces at the moment so I'm not very familiar with the latest options. Looks like rcconfig, newlineMaximum, are new and missing from the CLI and some others seems to be incorrectly named. Breaking backwards compatibility seems fine to me since it's a major change to include the CLI in core so developers will need to actively migrate.

schorfES commented

Hi @evanshortiss, thanks for your response. I think we should start with some feature issues in the lintspaces repo to create a roadmap for the upcoming changes with a CLI label.

Which minimal node version is required?

A short while ago I've already dropped the support for node 5 and added LTS/* as node version into the travis config. So I'm fine to go with LTS support at node 6.x.

Should we update the core code to ES6, if required?

I think that it is not required too. I also can't remember that lintspaces currently uses any asynchronous line of code, so there is currently not need for async/await.

Should we update the testing and validation setup? (nodeunit vs. jest? jshint+jscs vs. eslint?)

To have a test coverage would be nice. I prefer to migrate the exisiting tests and validation processes to jest and eslint. I will start this as soon as possible to have a basic setup for the next changes.

I'm currently still not a big fan of automated code changes, but I'm happy to see a working example.

How should the project file structure look like?

I think we should move the code into a src/ directory and place core and cli code in there at one place. What is required for a working CLI? Do we need an postinstall script which links the CLI code into the node_modules/.bin/ directory for all users? Does it work the same for Windows users?

Do we need a build/publish process?

Cool! Let's give semantic-release a try.

Do we want to be fully backwards compatible with options of lintspaces-cli? Which options to add?

Okay, the we should create a list of all options/flags we would support inside a corresponding feature issue. When planning this we could also add feature or fixes like #20, #21 and #23.

I'm on holidays for the next two weeks. So I will reply as soon as possible.

Regards, Norman

evanshortiss commented

@schorfES this fell off my radar. Do you still want to move this into core?

I can look into it after Christmas, and maybe some evenings during. I think maybe the best start is for me to update this current module dependencies, do #21 since it seems easy, and make then I make a PR into core?

schorfES commented

Yes, I still want to do that and I would be happy to have you with me. Mean while there have been many other things to be done so far.

I also want to create a new branch after christmas and start with some refactorings (codebase, file structure, tests, etc.). I also played in another project with a node-based cli and have an idea of what to do.

evanshortiss commented

Awesome, I just updated some stuff in this project with the PR to add --json support via a flag. So it will be easier to make a PR now and build on top of this since it can be tested.

schorfES commented

Nice, Ive already started a new brach next/tests to refactor validation (replace jshint and jscs with eslint) and tests (nodeunit vs. jest + coverage report). I will create a PR for you.

In the next steps I would like to refactor the code to use es6 classes instead of the prototypical approach for better readability, the constants and the general structure of directories of the project in a branch called next/structure:

  • rename lib/ into src/
  • add bin/ (new for cli, references src/cli.js or like that)