Thursday, March 31, 2016

Coaching and the PR Code Review

There is rarely a time when all the members of a team have a deep understanding of all the technologies being used on a project. On top of that the technologies that they're using are evolving and all team members need to keep up with the changes. Some members will learn new techniques and features earlier than the others.




An effective way of cross training team members is through the Pull Request (PR) Code Review.

Comments on a typical PR will be:
  • This looks like a mistake because...
  • This doesn't conform to our standard because... (This should rarely happen because a lint step should have caught this earlier. If it is happening then the lint rules should be reviewed.)
  • This works. I would do it this other way because...
  • Please add/modify a test for this.
In addition to that, as a reviewer, you should be asking:
  • I'm new to this technology, what does that do?
  • Please can you add a comment above this line explaining what this does?
  • I haven't seen this syntax before, is it the same as?
  • This is fantastic! I never knew you could... (Call it out when someone does something you haven't seen before or does something really well and you learned from it. Everyone loves positive feedback.)
If you submit a PR that has new features or techniques then consider immediately adding comments (in the PR) about them to help the reviewers if know that they will not understand what something does.

The question then arises, should I add a comment in the code or a comment in the PR? My take on this is that comments about how a language, framework or technology works, in isolation, should go in the PR comments. You don't want to clutter the code with information that can be found through a search on the web. The comments that go in the code are how the code works.

These are the advantages of learning through code reviews:
  1. The reviewer is learning in the context of the project domain. You don't get more real world than that. Not only will the reviewer be learning new syntax/constructs but they will also be understanding the business domain and gain knowledge in part of the code base that they may need to maintain in the future. Compare this to learning contrived examples in a class.
  2. The reviewer is learning a subset of the technology as it pertains to this domain. We would all love to learn all aspects of each technology we touch. Time constraints do not make this possible. Learning like this in situ provides the most time efficient way of learning the essentials. Again compare this to class learning where you may gain some super interesting knowledge and then not apply it.
  3. The coach, the person who submitted the PR, is forced to look at and explain their code. When you ask your seemingly naive question you may see a comment like "This construct in this language does... Now that I look at it again I see a problem/way-that-I-could-improve-it."
Using this technique you get a better code review on the PR and a great contextual training session at the same time.

Monday, November 23, 2015

MomentJS Notes

Install

npm i moment --save
npm i moment-timezone --save

Use

var moment = require('moment-timezone');

Create moments in and out of DST:

var cdt = moment.tz("2015-07-23 08:30", "America/Chicago");
var cst = moment.tz("2015-11-23 08:30", "America/Chicago");

Check that offset from UTC is what you'd expect:

cdt.tz('America/New_York').format()
> '2015-07-23T09:30:00-04:00'
cst.tz('America/New_York').format()
> '2015-11-23T09:30:00-05:00'

Check that zone aware format output is what you'd expect:

cdt.tz('America/New_York').format('HH:mm:ss')
> '09:30:00'
cst.tz('America/New_York').format('HH:mm:ss')
> '09:30:00'


Thursday, November 19, 2015

Node Foundation Membership Election

Mikeal Rogers recently posted Nominations for the 2016 Election for individual representation on the Node Foundation board.

I'm putting myself forward and answering the question "Why would you like to represent the individual membership of the foundation?"

Why?


I think that Node.js and the ecosystem around it plays a critical role in our current technology stack. This is going to become more important in the future.

I would like to be able to help shape the success of this platform. One way to do that is to understand what the membership wants and needs to get out of what we have today. More importantly what do they expect from the future. As part of the bridge between membership and the board I will be working to ensure that members' opinions are represented at the leadership level.

Qualifications


Leadership: In the past I've served as a director of engineering for a large IT company managing a team of 35+ managers and developers.

Technical: I have a few open source projects that I manage. I also contribute to open source projects I can improve through pull-requests. My day-to-day work is mostly writing JavaScript against the Node.js platform and system design and architecture.

Questions


I've intentionally kept this brief. I'll answer any questions. If you ask them as comments to this post then we can keep them in one place. Anyone else reading this will be informed.

Wednesday, November 18, 2015

Babel Cannot read property error of undefined

This is more a reminder to myself about how to solve this error that I've stumbled across a couple of times with Babel 5 & 6.

If you downgrade Babel from 6 to 5 and leave a .babelrc file in your project with a Babel 6 specific setting like:

{
  "presets": ["es2015"]
}


then you might get an error like this:

if (!option) this.log.error("Unknown option: " + alias + "." + key, ReferenceError);

TypeError: Cannot read property 'error' of undefined


The solution is to remove the .babelrc file or remove the Babel 6 specific settings.

Also, if you're using Babel 5 and you require a module from another project that has a .babelrc file which is using Babel 6 and has a Babel 6 setting in it then this will cause the same problem in your Babel 5 project.

Wednesday, November 11, 2015

Docker and GLIBC_2.14 not found

Hit a problem today when I was using Docker and Docker-compose to run a Node.js app in a CentOS container. I had done an "npm install" locally and then the docker-compose command had setup the container and copied everything over from my Ubuntu workstation.

Problem was that I was using a couple of Node.js modules that used native code. So the Ubuntu compile of those modules was getting copied to the CentOS container and when CentOS was trying to run them it was giving me an error about "GLIBC_2.14 not found" because it hadn't compiled those modules.

I feel that a good practise for almost any Docker setup is to have a .dockerignore file which excludes the "node_modules" folder.

This is what my .dockerignore file looks like:

node_modules/


Saturday, October 31, 2015

My Ideal Full Stack JavaScript Team Environment

Here are some of the tools and practices that I'm currently in favor of when working in a team environment on a JavaScript Full Stack project. In no particular order.

Pull Requests


All code should have a second set of eyes on it at some point. This serves the following purposes:

1. Catch bugs or potential bugs.
2. Teach both the reviewer and reviewee.
3. Keeps the team members up-to-date with what other teams members are working on.
4. Introduces best practices, new features/techniques and new libraries.

PRs should almost always be assigned to someone. Others can be @mentioned. Almost anyone (senior enough) should be able to signed off with a Looks Good To Me (LGTM) in the comments and merge.

The developer submitting the PR should let the reviewer know the urgency of the PR.

1. Urgent. The submitter immediately accepts the PR. The review is done post-facto as a learning and review process. Can also catch bugs that are fixed in the next PR.
2. Normal: Someone is expected to review this within an hour.
3. Low: Review within the next day.

Linting


An automated linting tool should be part of the commit process. Locking in lint errors to a maximum value will allow the gradual introduction of linting to existing projects.

My default (and currently the mostly popular) linting tool for JavaScript projects is ESLint.

Developers should be encouraged to have a lint helper plugin in their editors so they rarely/never see lint errors on commit.

The team should have an evolving .eslintrc file that's shared across all projects. Some projects might have specific override .eslintrc rules if they are catching up with the standard or have exceptions.

Keep the master .eslintrc in a dot-files folder that's a sibling of all the other projects and reference it from there.

ES6


ES6 is here. There are very few reasons not to start using this. Babel is an excellent transpiler that will allow developers to start using those features.

Use code reviews to help teach other team members ES6.

If you're not using ES6 you'll start leaking developers to other teams and companies. More on this below...

Hot Loading


Most projects should have tooling setup so that they can quickly iterate on small changes to code that they are working on and see the results.

This rapidly accelerates both learning and development.

If, for example, you're working on a React/Node project then, in my opinion, you'd have the Webpack Hot Module Replacement transpiling your changes in memory and making them almost instantly available in the browser.

As developers we're always on a learning curve. We can never be expected to know everything. Using a fast responsive hot loader we can quickly try different techniques and see the results. This will speed our learning of how that corner of the world works.

Front End Framework

Most projects should be using a modern front-end framework for the UI component. The most popular over the last several years has been AngularJS. The current rising start is ReactJS.

I'm in favor of ReactJS. It looks like AngularJS and EmberJS are converging on what ReactJS has introduced.

Latest Versions


We should strive, as much as possible, to be using the latest versions of everything. It's perfectly acceptable to hold off on the brightest and shiniest until it's stable. That should be a conscious decision and not the default.

Npm has a great command npm outdated --depth 0 that will give us a summary of how our project looks compared to what's available. Use that command with the -g option to check your globally installed tooling.

The 3 main reasons usually given for keeping up with the latest version (1. Features 2. Performance 3. Bug Fixes) are completely irrelevant. The reason you want to stay current is to keep your developers happy and engaged. Nobody wants to work on yesterday's technology.

Keeping a buzz in the team about moving to the next version will keep everyone up-to-date on what's happening and engaged with the team. The grass-is-greener syndrome with other teams or companies won't exist on the technology front because you'll be using the latest. In fact I've successfully used this as a recruiting tool for developers who are stuck in other companies or teams on old technology.

Related:

Your tooling should automatically keep your project's dependencies up-to-date with what would be installed when someone initially installs the dependencies on the project. I use the npm-update-outdated module as part of my pre-commit hook to update my dependencies before the tests are run so I know that my tests have run against what the CI server will use.

Project Onboarding


If someone asks you to work on a project then you should be able to do the following pretty quickly.

1. git clone
2. npm install
2. npm test (should run clean)
4. npm start

And have a server up and running that the browser can hit and interact with.

There are lots of complexities that might stop something that simple from working.

The project's readme should have bullet proof (as good as possible) instructions on how to address that.

For example, most developers have Docker. If you have a dependency on a MongoDB then npm start or npm install should run Docker and pull down a MongoDB container and get it up-and-running.

A new-to-the-project developer who experiences problems with an out-of-date README.md and who then updates the README.md should be given the rest of the day off as a reward. (That might be a bit extreme. This almost never happens so it might also be okay.)

Testing

All projects should have tests. These should run during a pre-commit hook. Ideally these are run implicitly through the Code Coverage tool.

When bugs are fixed a test should usually be written demonstrating the bug before it's fixed. This test will prevent a regression on this bug later.

Tests are difficult to write. Do lots of cross-education with other team members during PRs with tests. Get to know and love all the tooling around tests. When you're using a tool for the first time ask for help and opinion in the PR.

Some of the tools I use for testing:

Code Coverage

Code coverage such as Istanbul can be automated as part of a pre-commit hook. It will help make sure that critical sections or sections that you're about to change have sanity tests. It has awesome "graphics"

Code Coverage Minimum Levels


It's easy for a developer to commit new code that doesn't have test coverage. In my enthusiasm to get my new code out there I often forget to write tests for it. Using a tool like Istanbul we can prevent code coverage from dropping from the previous level in a pre-commit hook. This will remind us that we need to add tests.

Update 11/2/15


Isomorphism


I didn't used to think that isomorphic JavaScript was that important. I believe that it is now becoming important.

Least of all you should never have duplicate code that's duplicated for the sake of using on both server and client. There is almost always a way to provide the same code block to both client and server to provide a DRY and isomorphic code-base.

Tuesday, September 22, 2015

ReactJS JSX gives us HTML in JavaScript

What I consider the main difference between ReactJS and its JSX syntax when compared to the other frameworks like AngularJS, EmberJS, and BackboneJS is that JSX is HTML in JavaScript. The others have JavaScript in the HTML.

Here's a simple example:




React's JSX:

myCollection.map( (item) => {

  return render (

    <div>{item}</div> 

  );

});


AngularJS:

<div ng-repeat="item in myCollection">

  <div>{{item}}</div>

</div>

One of the things that we continuously struggle with is the ability to program markup. To do this we've been adding language to the markup.

JSX stands that paradigm on its head and adds the markup to the language.

As a developer I enjoy writing code more than writing markup. JSX means that I don't have to jump-out-of the markup to make things happen to it. The markup is encapsulated inside the logic that controls the flow of the application.

With AngularJS I'm consciously wiring language and markup together.

AngularJS appeals to the designer in me while JSX appeals to the developer in me.

I'm not against AngularJS or the other frameworks. I enjoy writing code and solving hard problems. I also hated the sight of JSX when I first saw it and before I wrote my first project in it. It didn't seem right to pollute JavaScript with markup.

For me the AHA moment was the productivity I felt that React gave me by keeping me in my area of expertise: JavaScript. I'm good at HTML and CSS. I'm an expert at JavaScript. More productivity happens in your area of expertise.

Long live ReactJS.
Long live AngularJS.