Linting

[martinhbramwell]

Linting

Did you see Sashko's esLint suggestion? https://forums.meteor.com/t/eslint-configuration-for-official-mdg-style/271/5 He isn't the first I have seen recommend AirBnB's coding standard, so I think it makes sense to use it.

However, there are many little details to consider :

• Maybe we should address only the boilerplate code at first. Code quality being an important aspect of Starrynight, it's self-contradictory not to ensure that the boilerplate, at least, has a consistent coding style.

• The quotes vs double quotes debate rages on. AirBnB goes for single-quote. It's your choice, of course! I think it does not matter all that much which you choose, but I think choosing does matter for the StarryNight boilerplate. If someone does a global search and replace with a pattern containing a quote or double quote, the results have to be consistent, right?

• Git diff leaves everyone pretty much stranded when reviewing the effects of a massive linting job on a file. You may want to think about how bad that would be &/or how to manage that. You can see it in my commit "A first try with esLint".

• How is StarryNight's test coverage? If a linting exercise on a file broke something, would your tests catch it?

• The main stylistic issues I have seen so far in StarryNight are :

  error    Multiple blank lines not allowed                                 **no-multiple-empty-lines**
  error    Expected space or tab after // in comment                        **spaced-line-comment**
  error    Missing space before opening brace                               **space-before-blocks**
  error    Missing space before value for key "click .addFooItem"           **key-spacing**
  error    All "var" declarations must be at the top of the function scope  **vars-on-top**
  

• Others are "defensive programming" defences, that make a lot of sense to me.

  error    {variable} used outside of binding context                       **block-scoped-var**
  error    {variable} is defined but never used                             **no-unused-vars**
  error    Missing trailing comma                                           **comma-dangle**
  warning  Unexpected console statement                                     **no-console**
  error    {variable} is not defined                                        **no-undef**
  error    Unexpected var, use let or const instead                         **no-var**
  error    Strings must use singlequote                                     **quotes**
  

• Can you see if there is a way to get eslint into your editor? Mine, in Sublime Text, highlights and describes all linting error and warnings. Please see this wiki page of my fork produced by running this command --

  eslint tool/generate-nightwatch-config.js
  

[awatson1978]

Martin, this is fantastic stuff.
In general, I'm on board with everything you're suggesting. It's stuff I've wanted to implement for awhile, but for lack of time, I've been concentrating on my area of specialty (healthcare, hipaa, fda) and hoping that somebody else would be able to eventually contribute some linting, logging, and other best practices. Suffice it to say that I'm in agreement with most all of it.

• Linting the boilerplate code is a must.
• I have no strong opinions on single vs double quotes.
• I thought that first try was pretty reasonable.
• StarryNight test coverage is rather limited right now. What tests there exist are implicit, rather than explicit. They basically consist of manually walking rough the tutorials in the examples page of starrynight.meteor.com; and verifying that the applications build correctly. It's better than nothing, but there's a lot more we can do. I have a best practice on now to test node command line apps which I've been following, and I went through a big refactor a month ago, to separate all the commands out into their own folders, with the intent of putting them into a library that can be loaded and tested with... gah, I forget it the libs... see parse arguments with minimist #25, split commands into separate files #26, decouple command logic into main library #35, and add event emitters to implement -verbose mode #37, which all relate to the following slidedeck: http://michaelbrooks.ca/deck/jsconf2013/
• basically, I've been refactoring the utility codebase to get proper testing added, but for now it's just manual scripts.
• no problems with the stylistic issues.
• will be interested to see some actual examples of the defensive programming techniques, because I do practice defensive programming. That's definately a thing for me. But that's sometimes for lack of the right tools.
• Atom definately supports linting. With this eslint file, I'll get things configured.
• I have family obligations later today, so will probably get to this stuff tomorrow.
• I want to pull the branch and run it before merging I to master. If I set up a develop branch, can you submit PRs to it?
• I think I'm on board with bunyan, but I'd like to set it up so there's not an extra installation step. The idea is to sort of minimize the piecemeal aspect. If I'm getting a new team member set up, I don't want it to be a 'gotchya', if possible. Maybe we could include it as a 'starrynight monitor' command?

[martinhbramwell]

Thanks! It's really gratifying that you approve of what I've done.

• Linting the boilerplate code is a must.

Good. As I get to know bits of it, I could submit linted versions from time to time. However, I would prefer to begin doing that after you have done two or three files. That exercise might lead you to adjust the rules, and then what I did would have to be redone. Can you find time for that :-p ?

• I have no strong opinions on single vs double quotes.

I was going to suggest we follow the Meteor Style Guide. Hah! They don't refer to quote handling at all, and worse, they use both single and double in one 10 line code fragment.

• StarryNight test coverage is rather limited right now. What tests there exist are implicit, rather than explicit.

I don't see myself tackling that anytime soon. Sorry.

• will be interested to see some actual examples of the defensive programming techniques,

I'm not sure what you understood me to mean by that point. Those seven lint rules each refer to a known practice to avoid. The defence is simply having esLint catch you doing it, rather than some bizarre failure in the future.

• Atom definately supports linting. With this eslint file, I'll get things configured.

Great! Like I said, I prefer to hold fire on any more work on this until you have settled down with a rule set you can live with.

• I have family obligations later today, so will probably get to this stuff tomorrow.

Yeah, and I need to focus on my presentation for next month.

• I want to pull the branch and run it before merging I to master. If I set up a develop branch, can you submit PRs to it?

Whatever you like better. I don't anticipate committing any more for a couple of weeks.

• I think I'm on board with bunyan, but I'd like to set it up so there's not an extra installation step.

I understand the concern, but I am pretty much a Node n00B, so I can't help much. Installing Bunyan is only really necessary for piping output to a more normal looking log. The HUGE advantage of bunyan is to be able to stream JSON fornat logs into permanent storage. Which raises the question of Kadira. It seems to be the Meteor way of doing things. Should we look at that, maybe?

[awatson1978]

No problem. Slow and steady wins the race. ;)

Linting and logging are fairly big contributions, in of themselves. I feel like I've tried a half-dozen linters and loggers over the years; but they've been very early releases, I didn't have a utility like starrynight I could roll them into, editor support was iffy, etc. That being said, I have a good feeling about the timing being right and all the pieces being in place to get this established in the pipeline.

So, yes, I'll find the time to settle on a linting ruleset (that will follow the AirBnB / Meteor Style Guide fairly closely), and will add some commands for linting and monitoring.

Anyhow, thank you again for prodding me to upgrade the linting and logging. They're much needed features.

And good luck with your presentation!