DEV Community: Brennan

Why Truly Great Product Managers Love Code Review

Brennan — Wed, 05 Dec 2018 20:41:28 +0000

Many years ago, I started my career in product management with only a basic understanding of what is code review—developers looking through each others' code for bugs. With upcoming deadlines, KPIs, user (re)engagement, compliance, and revenue goals all competing for my attention, understanding our development team's code review process wasn't a priority. After all, we had a great QA team. It took some time for me to understand that code review is so much more than identifying new issues.

We were working to ship critical updates to our new user signup flow, and every day without fixes had a real and meaningful impact on our bottom line. My plan was to push changes to our staging environment on Wednesday, have QA review it on Thursday, fix issues through Monday, and launch to production on Tuesday. Our team was motivated, and we got to work.

Come Wednesday, the expected changes were glaringly absent from staging. Our development team found several launch blockers in code review, and they were still working to resolve them. Fast forward to the following Tuesday, and there were still no changes on staging. I was furious to learn that the code had been reviewed three times, and the last build passed all of our unit testing.

I thought, "If the code is functional, then what else really matters? It's only code. Why are my engineers wasting time reviewing code that works?"

Functional code isn't necessarily high-quality code

Sometime later I inherited an application burdened by years of technical debt, with zero documentation and no access to the original developers who wrote it. Attempting to update a button's position caused the app to fail spectacularly, and no one could explain why.

But despite all of its poorly written functions in several different languages patched together using obsolete and unmaintained libraries, the app technically worked. It was functional and passed QA, yet no one wanted to work on the project, which should have been my first clue.

I originally scheduled one week for our first sprint to make a handful of maintenance updates. It took eight.

And the toll it took on our development team was even more costly. One of our developers felt the need to apologize: "I'm sorry this is taking so long. I'm just having a lot of trouble moving this one button."

For a developer who takes pride in her work, eight weeks of trudging through a swamp of technical debt is crushing. As a PM, you know the concrete costs that come with each day your fixes aren't yet live, and there are also the intangible costs to your development team—a loss of trust and faith in you as a leader, in the project, and in the team to get things done.

Not to mention the opportunity costs. Eight weeks spent spinning our wheels working with bad code was eight weeks we all wished we could have spent shipping features our users actually wanted and needed.

Healthy, high code quality is key

In addition to identifying issues, code review is also about improving code quality and maintainability. While unit testing is great at verifying that things work the way they're supposed to, it's not as adept as people are at offering suggestions to simplify complex functions, ensuring code is human readable, presenting alternatives for upcoming deprecations, or verifying automated tests are set up correctly.

For example, imagine a simple app to charge customers for an item. The code requires a currency to be declared, and in this test case, it should be in USD. However, the currency's default value is set to EUR and was never changed. The code will have a high likelihood of passing unit tests, and if the currency unit isn't displayed on-screen, it will likely pass QA as well. If this bug went live, the potential cost to your business would be enormous. These are the kinds of issues that can be discovered through a rigorous code review process.

I've learned the following lessons through my experience leading development teams:

Be an advocate for improving code quality

You have a responsibility to your team and to your users to produce features that are safe, secure using high-quality code. Account for code review in your sprint planning and make it a priority, not a "nice to have" that can be punted.

Understand major issues identified in code review

Whether you're a technical or non-technical PM, take the time to understand major issues, proposed solutions, and timing. You're responsible for all aspects of your products. This will help you avoid running into the same or similar issues in the future.

Support your development team

Starting or maintaining a healthy code review culture can be challenging, but you owe it to your team (and to future teams that will maintain their code) to establish good practices now. Helping the leaders on your development team to set on the right path today will pay dividends going forward.

Invest in code review

Code review is a low-risk, high-yield investment. It's easier, faster, and cheaper to identify issues in code review than through user reports posted, forums, social media, or even a massive, costly security breach. Code review as part of a healthy regiment of development practices enables your team to ship meaningful features to your users faster, and with fewer issues in production.

This was originally posted on PullRequest's blog. Get your code reviewed by professional reviewers: Learn more about PullRequest

What It Takes to Get Certified to Review Code At Google

Brennan — Wed, 14 Nov 2018 19:09:40 +0000

Many PullRequest code reviewers have experience at big tech companies, like Facebook, Amazon, or Netflix. The following Q&A with one of our reviewers describes their path to getting their certification to review code at Google called "code readability."

How does code review at Google work?

In order to push JavaScript to production at Google, you're required to have a code reviewer with JS readability to review and approve the code. Readability is an internal certification that shows you understand Google's coding style and best practices for a specific language. Every CL must be written or reviewed by an engineer with readability in that language. Period.

Our team didn't have a reviewer with readability, so we always needed to ask other teams to review the code on our behalf. As a hard requirement, this blocked us from pushing changes and hurt overall velocity.

To solve the bottleneck, our tech lead and I started the process to get JS readability, which is like going through a sort of "code review code review."

What's a "code review code review?"

In order to apply for JS readability, engineers submit CLs to a team of readability reviewers who go through the code with fine-tooth combs. An assigned reviewer makes comments and approves the code only after you've proven you understand Google's style guide and best practices.

What was it like to get your Google code reviewer certification?

I submitted an Angular component to make a calendar for an internal tool. It came back with red ink all over the place. Every little thing from how many spaces need to be above constants, to removing extra spaces in these function declarations.

How was the reviewer pointing out details different than a regular code review?

Unlike a traditional code review, the readability process is the one time where the assigned readability reviewer holds nothing back. Every single minor thing that could possibly be pointed out, will be. But in the end, going through the readability process made me a better programmer and code reviewer.

It was a proud day for everyone when I got my official stamp of approval to review and approve any JavaScript at Google. I received congratulatory emails from everyone on the team because they knew how long and how challenging the path to JS readability is.

What's it like doing code review at PullRequest after reviewing code at Google?

What's right for Google doesn't necessarily mean it's right for every team. I learned a great deal about the code review process and why readable code is so critical through my path to JS readability. I keep the spirit of these lessons learned in mind when I review code for other teams, and focus on being positive and as constructive as possible.

This was originally posted on PullRequest's blog. Get your code reviewed by professional reviewers: Sign up today for PullRequest

Code Reviewer Spotlight: Dylan Drop

Brennan — Mon, 05 Nov 2018 13:05:00 +0000

A good code review process can be the fastest feedback loop for developers to receive actionable feedback from senior software engineers. PullRequest reviewer Dylan Drop also sees code review through the lens of mentorship, and emphasizes that developers and reviewers share a common goal of delivering high-quality code.

Dylan Drop

Our Code Reviewer Spotlight is an ongoing series of interviews so you can get to know our top reviewers. Learn more about Dylan’s drive for continuous learning through code review below:

1. What's your background?

I've been doing web programming since 2011. I used to work a lot with Rails and Ruby, especially in my last role as a tech lead at Blue Apron. I helped scale the company from 12 engineers to 100+.

I'm currently a technical lead at Grubhub and I'm working more with Java now, but will always hold a spot in my heart for Ruby. In fact, I used to contribute to crystal-lang, which is a Ruby-like language that is compiled, statically type-checked, and has speeds similar to C.

2. Why is code quality important?

When you're writing code, you usually just want to get the job done, but what's even more important is making sure it behaves in a way that's easy to understand, extensible, and side-effect free. If your code is high-quality, you'll be sure that all those boxes are checked. You make your life easier for your future self (and your colleagues).

3. Why is a good code review process important?

Put simply, code review makes your code better. It's a kind of mentoring process that makes your change being reviewed higher quality, and also helps you develop and grow as an engineer.

Reviews can sometimes feel frustrating if the reviewer finds mistakes or improvements you hadn't considered. However, if the code review makes your change higher quality, you'll thank yourself for it later. Both you and your reviewer are on the same side, working together to deliver high-quality software to the user.

4. Why do you review with PullRequest?

PullRequest allows me to get back into the Ruby world and keep myself up to speed. My work at Grubhub is in Java, so code review ensures my Ruby skills stay current. Also, I get to see code from all types of business applications outside my particular industry; it's always interesting to see the inner workings of other tech companies.

5. What advantages have you found of being a third-party reviewer?

Oftentimes teams can get bogged down in their set ways. It's good to have another party (like myself) come in and introduce them to possible improvements they wouldn't have otherwise considered. Each person brings their own background and expertise to the table, and teams are then able to make better, more informed engineering decisions about their end product.

6. What are some common issues you see?

The number one thing I see is lack of understanding around database properties and how the application code will behave in those scenarios. Developers need to understand transactions, isolation, atomicity, and all the other properties the database gives you, and what will happen when things go wrong.

7. What advice do you have for developers?

Improve your understanding of the inner workings of whatever frameworks or technologies you're working with. If you're using an ORM, understand what the magic behind the curtain is doing. If you're working with a message queue, understand if you're guaranteed at-least-once delivery, and what happens when a message fails to be processed. Doing so will save you from a lot of headache down the road, and you'll be all the better for it. I guarantee it will benefit your career.

Sign up for PullRequest and have vetted, professional reviewers like Dylan work with your development teams to ship high-quality code.

The $85 Billion Cost of Bad Code

Brennan — Wed, 31 Oct 2018 17:00:00 +0000

A recent study from Stripe and Harris Poll found the average developer spends 42% of their time dealing with technical debt and maintenance issues, of which 3.8 hours are spent just on debugging "bad code," or poor quality code that's difficult to maintain. The opportunity cost of bad code comes to $85 billion annually — resources that could otherwise be used to build better software.

Establishing a healthy code review process can help address these issues by reinforcing the need to write readable, maintainable code. We've talked about some of the benefits of code review on our blog, but Stripe's study brings into focus the economic impact of bad code.

Companies can help their development teams by considering the following:

Understand your total costs. 59% of developers strongly agreed their companies spend an excessive amount of time fighting with bad code. Tech debt needs to be addressed, and you can leverage a code review as a service or outsource support to complement your development teams. When evaluating the costs and benefits of bringing on added support, remember to account for the opportunity cost of tech debt and maintenance issues, as well as the negative impact on team and personal morale.
Hire additional development support. The Stripe study isn't asking if you can afford to hire more developers — it's asking if you think you can afford not to. Writing high-quality code takes time, and with upcoming deadlines and competing priorities, 79% of developers felt they weren't given sufficient time to fix poor quality code. One way to responsibly scale your development resources is to use our network of on-demand reviewers.
Prioritize your developers' time. Lastly, not all developers enjoy or excel at maintenance tasks. Specialization can help teams become more efficient, and it can also lead to happier, more productive developers. Work with your development team to identify the right mix of time spent writing new code and time spent maintaining it.

This was originally posted on PullRequest's blog. Get your code reviewed by professional reviewers: Sign up today for PullRequest

ABC's of Public Relations (PR) for Engineers

Brennan — Thu, 11 Oct 2018 17:00:00 +0000

PR (public relations) can be an effective way to drive a lot of awareness around what you're building, and help your team increase signups, lift sales numbers, or generate interest from investors. And unlike ads you might pay for, PR is earned media, which is free, and can feel more authentic because it comes from an unbiased source.

This is a tactical beginner's guide to PR for developers, meant to help you get started with your press outreach and PR strategy. While we work more with the other kind of PR (pull requests), the following four steps have helped us secure popular articles and blog posts:

Create your hook
Assemble your press kit
Identify relevant journalists
Pitch your story

Create your hook

The press care more about stories than they do about products. It's one of the reasons why you can build something great and still fail to get user traction. Unlike in the movies, if you build it, it doesn't necessarily mean they will come.

Tech specs, benchmarks, and research data are important and will help add color, so keep them in your pocket. But start with your hook — that story that draws people in and gets them to root for you. In simple but not condescending terms: what are you building, why does it need to be built, and why is your team the best team to do it?

Another way to approach this is to write your headline first. What's the title of the article that you want to see at the top of NYTimes and Hacker News?

If you can craft a story about the incredible event that inspired you to tackle this problem, or that time you ran out of funding but saved the company with a non-obvious insight, or how your product made a meaningful impact on a customer's life, then you'll raise your team's chances at getting covered by journalists and bloggers.

Stories that people connect and identify with can help turn strangers into friends, and friends into evangelists. You still need to build a great product, but a compelling story or mission will help your users feel like they're a part of something even greater.

Assemble your press kit

While you can't write your own stories for press outlets to publish, you can help them, help you by giving them the following resources:

Company overview: Create a one-page doc that includes your company name and tagline, a brief description of your product or service, a summary of how it works, your hook, and a call to action that tells readers where to go to sign up, make a purchase, or learn more.

Founder photos: Collect your favorite headshots, candid photos, and team photos with easily readable filenames, like lyal-avery.jpg or pullrequest-austin-office.jpg.

Logos: Include high-res company logos and app icons.

Press mentions: Create a bulleted list with links to other articles and blog posts that have been written about your company.

Product screenshots: Take high-quality screenshots on desktop, Android, and iOS using team members' accounts or dummy data. If you have links to product demos or marketing videos on YouTube, include those as well.

In a nutshell, make it as easy as possible for press to share your story with their audiences. Giving them high-quality assets that they can include in their articles will make their stories and your product look better and more professional.

Identify relevant journalists

Build a virtual Rolodex of journalists who are already writing about your category or focus area. Go to Google News and search for your company name, your competitors, and your industry. Open the relevant articles, then navigate to the journalists' profiles to record their contact info.

Yes, this is a time-consuming and manual task, but finding the right, relevant journalists will increase the likelihood that the people you connect with will actually write about your team.

Some journalists will list their email directly on their articles or on their publication's profiles, like Luke Stangel. Some will add their email to their Twitter bios, like Ron Miller. And some won't include an email at all, but can still be reached via Twitter DMs or LinkedIn Messaging (but tread lightly).

Create a spreadsheet with names, contact info, and publications, and remember to include enthusiasts' tech blogs, hyper local publications, and podcasts. Targeting smaller outlets is a good strategy if you're just starting out, so cast a wide net.

Pitch your story

Finally, send lightly personalized emails to your list of journalists, and update your spreadsheet to keep track of who you've emailed and when you last contacted them. Start with a brief introduction to establish credibility, give your pitch, then make your request.

Here's a sample email that worked well for our team:

Code review as a service with PullRequest
Hi [NAME], I'm Lyal, Founder and CEO of PullRequest, and we're building a platform for code review as a service that pairs best-in-class automation tools with a network of on-demand reviewers. I think your readers will really enjoy learning how we went from Y Combinator to funded by Google, and I'd love to send you our press kit and tell you more about what we're working on. Thanks, Lyal Avery Founder & CEO, PullRequest [CONTACT INFO]

Code review as a service with PullRequest

Hi [NAME],

I'm Lyal, Founder and CEO of PullRequest, and we're building a platform for code review as a service that pairs best-in-class automation tools with a network of on-demand reviewers. I think your readers will really enjoy learning how we went from Y Combinator to funded by Google, and I'd love to send you our press kit and tell you more about what we're working on.

Thanks,

Lyal Avery
Founder & CEO, PullRequest
[CONTACT INFO]

Following these four steps should help you with your press outreach, as well as help you explain what you're building in a way that resonates with more people. There isn't one magic formula that will guarantee every journalist and blogger will cover your next product launch or funding announcement, but hopefully this gives you a starting point that will help you design the right PR strategy for your team.

Keep reading, this post is part of a four-part series that includes Product for Engineers, Marketing for Engineers, and Sales for Engineers. Get your code reviewed by professional reviewers: Sign up today for PullRequest

Read Me First: An Intro to Internal READMEs

Brennan — Wed, 05 Sep 2018 13:45:24 +0000

The most successful open source projects have informative and entertaining READMEs. There are badges, screenshots, animated walkthroughs, and getting started guides. Some even break it into separate documentation sites that match the sections in a README.

Prettier's through README on GitHub

With a quick glance through one of these open source repo’s README file, you’re likely to have a complete grasp of the project. You’ll be able to access a summary of the purpose, who made it, why they created it, who it’s for, how to install it, and how to contribute.

It’s usually not the same as the crucial repositories inside of our organizations. It’s odd these often end up with outdated and empty READMEs despite being mission-critical components of a business.

Are OSS maintainers obsessive documentation fanatics?

I don’t believe maintainers and contributors have a bizarre affinity for documentation.

They are passionate about creating software that helps others change the world. To get their project into the hands of the right users it takes some
'marketing.'

When you dive into a new popular project, it’s clear someone took hours of time to document it so more people could use and contribute to it.

After volunteering hundreds of hours of work, it makes sense they’d want it to be as easy as possible for people to understand, use, and contribute. Without a thoughtful README, potential users would likely not discover the repo and miss out on the great functionality.

Does this mean every repository in your company should have an equally robust README?

One with thousands of words, build stats badges, and GIF walkthroughs?

No, your team’s codebase is different from open source. There are only a few potential contributors to your repos, you’re likely all in the same office, and there are opportunities for people to ask questions if they get stuck.

It’s likely most of the code only applies to your business’s use case and despite an empty README your team will still use and contribute to the project.

It makes the README an afterthought for most internal repos. Something on the list of things to do between sprints. They're usually filled with vague bits of information added when the repo was created long ago.

Many dev teams think since they’re not trying to market the repo to the world there’s no audience at all for an internal README.

Example of a standard internal README.md

So why bother with spending time on your READMEs? Fundamentally, having a useful README makes it easier for your team to push cleaner code.

Everyone on a development team benefits from a project with a clearly defined README. New hires will be able to quickly get set up contributing code that matches the team’s standards. Overall, the whole team benefits from having more people know how to run and fix the project.

Here’s to small changes in your READMEs

Most internal repositories contain the bare minimum already, such as a single sentence describing its usage. While there might be a lot that could be added, it’s difficult to know from the beginning what’s vital to include for your company and the project. Don’t expect to write pages for every README overnight.

To start making improvements, there’s an easy way to set the foundation that gets the team to begin the process. Pick your three most important repositories and the three most essential things for your team to know. For example, how to run, how to test, and why critical decisions were made.

From that foundation, you can encourage everyone on your team to work towards incremental changes to the README whenever possible. Adding a small comment during code reviews of relevant pull requests will improve the overall quality of your READMEs.

This was originally posted on PullRequest's blog. Sign up for PullRequest today and get code review from professional reviewers backed by automation to help improve your code quality.

Seven Habits for a More Toxic Code Review Culture

Brennan — Fri, 17 Aug 2018 13:45:24 +0000

Please don't practice any of the following "advice." From junior developer to CTO of a Fortune 500, we're all guilty of making mistakes within development. We've picked the worst examples we've seen around code review and pulled them together into one terrible, awful reviewer:

Look — I get it. You've done everything you could to ensure that no one would ever ask you to do a code review, but they've gone and promoted you anyhow. Now you have a whole team to bring down with you, down to the depths of a toxic code review culture.

This won't be easy — making enemies never is — but if you follow these seven highly effective habits, no one on your team will even think about submitting a pull request. And less pushes means less work. Win-win.

1. Just the facts — opinions are for lesser developers

Don't waste time leaving a comment that isn't a straight fact. Remove softening language, like "I feel," and never use "we" (unless you're being passive aggressive). Adding, "Functional programming was passé last quarter, Brent" is sure to bring out the best in all of your teammates.

e.g.

2. Ask for architectural changes on styling pull requests

They're already in there, right? Don't let them be lazy, let's tweak the database abstraction to handle an edge case you discovered on Klingon UTF support. Wait a minute... they aren't adding tests for things that have nothing to do with the pull request? Better suggest those too.

e.g.

3. Remember ABA — always be attacking

Catchy acronyms about abusing others are no longer just for Sales. Any reasonable request can be improved by closing comments with "stupid" or "you moron" or "why did you write this?" Call out their apparent lack of knowledge. Bonus points for referencing that obscure, esoteric article about handling an adjacent but unrelated problem.

e.g.

4. All aboard the bandwagon

I got a fever, and the only prescription is using hacky, buzzworthy solutions I barely understand. Insist the author refactor to them. Because who knows? Maybe their implementation will help you with your side project that stalled.

e.g.

5. Tolstoy's got nothing on you

Concise comments are overrated. What people admire and respect are loopy narratives that rival in length most Russian Realist epics. Try and sneak references to what you ate for breakfast and hope no one catches it. Bury valuable feedback under the dribble so you can point to it after the merge and complain bitterly that no one ever listens to you. Scrambled eggs.

e.g.

6. Aged like A Fine Wine

The best things in life take time, including your reviews. Make sure that developers and product sit blocked while you finish your much more valuable activities. Those memes aren't going to share themselves.

e.g.

7. It looks good to me

Let's face it. You've skipped the previous 6 items. Time to pull out the ol' classic — looks good to me. Why do we have QA if we aren't going to keep them on their toes?

e.g.

Obviously, following even one of these habits can lead teams to certain doom, but we've seen shades of these traits in our own experience working with thousands of teams. We're here to help developers form good habits that will improve code quality. Get in touch today.

Thanks to Adam Nemecek for reviewing an early draft of this post.

This was originally posted on PullRequest's blog. Get your code reviewed by professional reviewers: Sign up today for PullRequest

Using FOSSA for License Compliance

Brennan — Thu, 09 Aug 2018 13:45:24 +0000

Technical leaders are often caught by surprise during diligence on a round or exit: the list of asks is long, daunting, and seemingly designed to slow down the process. There's one diligence ask that I have seen derail rounds and exits that's not nearly as hard to manage as it may seem... the developer tool FOSSA

Projects are usually littered with code from many sources. From importing via package manager to copy and pasted from Stack Overflow (a very risky practice, as establishing a chain of ownership is nearly impossible), developers add dependencies in a variety of ways.

For an acquirer or a venture capitalist, this is problematic. As an example: Facebook's original license for React included a reasonable IP assignment that could theoretically prevent a company for suing Facebook for anything related to patents (they have since re-licensed it to MIT). For a startup, this is a fair trade off: the likelihood of ever engaging with Facebook on a legal scale is low, while the velocity gained from using React is high.

Even items like packages licensed with certain GPL versions means an unacceptable risk for corporate compliance teams.

Historically, this meant all-hands going through the code base, refactoring where code authorship can't be established, or hiring an external consulting firm to do it for a substantial amount of money. Over my last exit (and our Series A financing here at PullRequest), I found a better approach with much less friction.

Step 1) Sign Up for FOSSA

GitHub, Bitbucket, and GitLab integrations are available, as well as manually uploading an archive of your project if you host your code in other places. Make sure to pick whether you're distributing code or offering a web service, as FOSSA will suggest a sane set of rules to check for within your project.

Step 2) Run Your Code Through FOSSA

Wait for the inspectors to run; a successful run will be pinned to the branch or version of the project that you have manually uploaded.

Step 3) Generate A Report

A list of compatible, non-compatible, and action items for adherence will appear. Most legal firms will want PDFs of these pages; they are helpfully print formatted and make an excellent combined file when complete.

That's all there is to it. While this won't completely replace a strict monitoring of your development team for bad behaviors (for instance, copying and pasting the entirety of a Stack Overflow answer), it will save you a ton of time during diligence.

At PullRequest, we recommend making this part of your workflow. We attach the resulting badges to each of our repositories, that way we know if we've introduced something that's incompatible with our targeted policy.

Here's an example license badge for one of my open source projects (a PHP wrapper for the background search service Checkr)

This was originally posted on PullRequest's blog. Get your code reviewed by professional reviewers: Sign up today for PullRequest

3 Principles to Get Your Pull Request Reviewed Faster

Brennan — Sat, 07 Apr 2018 13:45:24 +0000

How long does it take you and your team to get a pull request reviewed, accepted, and merged? How long should it take?

In my experience, the turnaround time a pull request takes to get through code review ranged from minutes to days. I’ve even had PRs sit for weeks waiting for code review. There are a few fundamentals I’ve learned that can help you get your code reviewed quickly and improve your pull request turnaround time.

Start with clean code

Writing code that’s readable and easy to understand speeds up the code review turnaround time of your pull requests. Your reviewer and future contributors should be able to understand what your code is doing just by reading it. In addition to well-written docstrings (where applicable), make a conscious effort to ensure your variable and function names are self-descriptive and try to limit methods to performing only one function.

Readable code is helpful beyond the code review process. In most codebases, the majority of the time someone reads code is by you and your coworkers to debug and refactor long after it was added.

Have you ever started a review on what looked like a massive pull request to discover it included a bunch of files that should have been in the .gitignore? Keeping the .gitignore up-to-date is an easy, but often skipped way to make your pull requests easier to digest for your reviewer.

To get ideas of what to include, check out GitHub's repo of .gitignore templates.

Just because your whole team knows that they don’t need to review the generated files or dependences, there’s no reason they should be tracked and submitted with pull requests. A quick update to a few lines can reduce bloat on every pull request for every contributor to the repo.

Limit the change size of your pull request

The most common culprit of lengthy code review turnaround times are pull requests with an immense set of changes – be it in actual lines of change or complexity. Pull requests created with the intention of introducing a few simple changes can suddenly grow out of proportion with several significant changes that are difficult to review.

Include code review time during planningThis PR grew from a few bug fixes to 1,255 LoC changed and sat waiting for review for a week.

Even when I'm watching for it in a pull request, I'll end up combining a feature, a few code refactors, and a bug fix or two that could have easily been split up into several more manageable sized pull requests. Teams with strong code review cultures encourage these smaller, more tactical, pull requests instead of large changesets. Sometimes this requires additional work, but your team's overall productivity (and sanity) will thank you.

However, tread carefully. This is an art that takes careful thought and practice. While it helps to break apart changes, excessively splintering your codebase into hard-to-follow fragments is no better.

While there is no number of perfect lines of code change for a pull request, I suggest thinking about how many lines and how much complexity you are including when you’re working on a branch. Consider the person reviewing the code and the language of the change.

Depending on the complexity of the code, and the situation, 300+ lines of code change might be large or small.

For new projects or startups, they tend to have substantial pull requests since getting a feature out is a higher priority than stability. Unfortunately, when this happens, and the project starts supporting paying customers, these teams end up spending much of their engineering time fixing bugs and refactoring code.

It’s far more expensive to fix bugs in production than it is to catch them during a stringent code review process backed by smaller pull requests.

Build a code review culture

An essential component to efficient code review comes from a team’s culture and its respect for code review and code quality. Building and maintaining this culture is no small undertaking.

You can’t build code review culture by commandments carved in stone (or a wiki) for all to follow. Small nudges and course corrections in the right direction at every stage of software development build culture. The importance of code review needs to be set by and demonstrated by the engineering manager and other leaders on the team.

Code review is a place for objective collaboration and encouragement, not judgment. It’s vital that your team’s sentiment towards code review is positive.

Developers should look forward to pull requests as an opportunity to learn, not dread them as an obligation to be vulnerable to an assault on their development skills. You can help this by making a habit of calling attention to well-written code in pull requests and if unproductive judgment sneaks its way in, make a point to address it quickly.

It takes consideration and time during development to achieve the type of maintainable codebase that comes from quality code reviews. To get quality feedback from code review, you should start with planning. Time for code review should be a consistent item of consideration for every sprint planning.

Include code review time during planning

If you don’t account for code review time in planning, engineers tend to focus on shipping their features first and reviewing their peer’s code second. This lack of focus on other features causes a delay in their teammates getting the useful feedback they need to iterate on from code review and slows the whole team down.

While accounting for code review in sprint planning leaves less room for additional features, a cleaner codebase with fewer bugs pays dividends in future productivity. All feature release stakeholders, including non-technical product and project managers, should consider and support code review.

This was originally posted on PullRequest's blog. Get your code reviewed by professional reviewers: Sign up today for PullRequest

SSH Login Notifications in Slack

Brennan — Sun, 21 Jan 2018 13:45:24 +0000

It's handy to know who's logging into servers around your projects. Slack offers a beautiful way to do this in combination with pam.d.

We're assuming you're using a CentOS-derived OS for locations, but this should work on any *nix-based OS with pam.d enabled.

1. Add an incoming webhook in Slack -- navigate to:

https://YOUR_DOMAIN.slack.com/apps/manage/custom-integrations

We recommend naming the spot something that is recognizable; that way it won't get deleted in the future.

Make sure to copy the Webhook URL from the resulting page.

2. Add an SSH script to your server

Add and make executable (chmod+x) file to /etc/ssh/scripts/sshnotify.sh

(note Make sure to replace <YOUR SLACKWEBHOOK> with the URL from step 1 and #channel with the channel you want notifications going to)

if [ "$PAM_TYPE" != "close_session" ]; then
        url="<YOUR SLACK WEBHOOK>"
        channel="#channel"
        host="$(hostname)"
        content="\"attachments\": [ { \"mrkdwn_in\": [\"text\", \"fallback\"], \"fallback\": \"SSH login: $PAM_USER connected to \`$host\`\", \"text\": \"SSH login to \`$host\`\", \"fields\": [ { \"title\": \"User\", \"value\": \"$PAM_USER\", \"short\": true }, { \"title\": \"IP Address\", \"value\": \"$PAM_RHOST\", \"short\": true } ], \"color\": \"#F35A00\" } ]"
        curl -X POST --data-urlencode "payload={\"channel\": \"$channel\", \"mrkdwn\": true, \"username\": \"SSH Notifications\", $content, \"icon_emoji\": \":inbox-tray:\"}" "$url" &
fi
exit

3. Add the script to your pam.d

sudo echo "session optional pam_exec.so seteuid /etc/ssh/scripts/sshnotify.sh" >> /etc/pam.d/sshd

4. Verify the installation

Log out and log back into your box to verify a notice hits your channel of choice.

3 Ways Code Review Makes Your Dev Team Better

Brennan — Tue, 09 Jan 2018 16:00:00 +0000

These three benefits of establishing a healthy code review process can help development teams write better code and do it more efficiently:

Improve readability and maintainability

With the exception of solo engineers working on their own projects, most software is a collaboration built by teams of people working together. As development teams grow, oftentimes the developer now assigned to maintaining a feature isn't the same person who originally wrote it.

Code review is the best opportunity to get feedback on the readability of your code from another engineer. In addition to checking for quality and performant code, a good review process encourages teams to optimize code for readability.

More readable code is also more maintainable code, which makes it easier to bring new team members up to speed, fix bugs, and deliver new features.

Identify issues before your users do

Like unit testing and QA, code review also helps to reduce defects in your codebase, but it happens much earlier in the development cycle. This causes less back-and-forth churn than letting issues make it all the way to your QA team, or worse, to your users.

With code review, you can zero in on a limited set of changes at a given point in time. After code has been released to production, fixing bugs reported by users typically involves looking across all of the cumulative sets of changes since your last release.

There are also certain types of issues that are easier to identify during code review. An experienced engineer trained to look for security issues can prevent vulnerabilities from being introduced in ways that would be much harder to catch otherwise.

On my engineering team, I've seen reviewers flag issues during code review that likely saved us cumulative weeks of time had they made their way into production. Code review can't guarantee perfect software, but it's the most efficient form of quality control we use as part of our development cycle.

Keep your skills sharp

If you skip the code review process or don't take the feedback seriously, then you're missing out on one of the shortest feedback loops in software development, and an excellent way to learn how to be a better engineer and build your knowledge base.

As an engineering lead, the mark of a great senior engineer is someone who's open to learning from feedback, who can also provide that level of guidance and mentorship to others. Establishing a positive feedback loop through a healthy code review process enables teams to grow and learn from one another.

This was originally posted on PullRequest's blog. Get your code reviewed by professional reviewers: Sign up today for PullRequest