Reviewing Code More Effectively

Blog Detail

I believe that it’s important not only to do code reviews when developing software, but also to really understand why we do code reviews, how to make code reviews useful and effective, and what to look for in a code review.

Why Do We Do Code Reviews?

Fundamentally, a code review serves to answer three questions, loosely in descending order of importance:

  1. Is a proposed change to the code a net improvement to the codebase as a whole?
  2. How might the proposed change be improved upon?
  3. What can the reviewer(s) (more broadly, the team) learn from the proposed change?

I want to especially draw your attention to the nuances of that first question. Code is never perfected, and expecting perfection in a code review can be counterproductive and frustrating to the contributor. The goal of a code review is not perfect code, but better code—the code can always be further improved in the future if it proves necessary to do so. Keeping that in mind can make for more efficient reviewing.

How Do We Make Code Reviews Effective?

As a maintainer of a software project, you should prioritize setting contributors up for successful code reviews. The details may vary depending on the size of the project and its pool of potential contributors, but ideas that generally apply include:

  • Labeling and categorizing open issues (bugs, feature requests, etc.) helpfully, especially in terms of their complexity or any specialized expertise that might be needed to tackle a specific issue. (Many open-source projects use a label such as help wanted or good first issue as a way to highlight issues that would be well-suited for a new contributor to take on, as one example.)
  • Documenting expectations for code contributions clearly, through tools such as GitHub’s “pull request templates” and CONTRIBUTING.md, as well as by providing well-written developer documentation in general.
  • Automating any part of the project’s requirements that can be effectively automated, including unit and regression tests, but also extending to tools such as linters, spell-checkers, and code autoformatters.

As a contributor to a software project, key points to keep in mind include:

  • Solve one problem at a time—the smaller and more self-contained a code review is, the more easily and effectively it can be reviewed.
  • Provide a thorough explanation of the reasons behind the proposed code change—both as code comments and as the “description” or “summary” attached to the code review request, which can and should include screenshots, example payloads, and so forth.
  • Provide testing to demonstrate that the change does what it sets out to do. (Ideally automated, but even a well-documented manual test is far better than nothing!)
  • Approach the code review as a learning experience, and take feedback with an open mind.

As a reviewer of a code review, you should:

  • Approach the code review as both a teaching experience (sharing your hard-won expertise with the current code) but also as a learning experience.
  • Provide feedback politely and without ego (no matter how tempting it may be to regard your own existing code as impossible to improve upon!).
  • Link to relevant documentation and best practices to clarify and support any feedback you provide.

What Should We Look For in a Code Review?

I like to think of different approaches to a code review as a series of distinct frames of mind, or “hats” that I might “wear”. You can also think of “wearing a hat” as assuming a different persona as a reviewer, then focusing on areas that are important to that persona. In practice, there are no firm dividing lines between these, and I’ll often “wear” many “hats” at once as I’m doing a code review, but it can be a useful checklist to keep in mind for thoroughness.

The hats that I’ll discuss here are:

  • Beginner
  • User
  • Developer
  • Tester
  • Attacker
  • Maintainer

Hat of the Beginner

This involves an approach often labeled as “beginner’s mind” in other contexts. Fundamentally, the goal is to approach the code without preconceptions or assumptions, being unafraid to ask questions and seek clarification. The key skill here is curiosity. For example, you might ask:

  • Is the code understandable and well-documented?
  • Does this code actually do what the function name, comments, docstring, and so forth imply that it should do?
  • What might happen if this conditional logic check evaluates as False rather than True?
  • What might happen if a user provides “weird” or “invalid” inputs?
  • All in all, does the code change “make sense”?

Hat of the User

When wearing this hat, you focus on the experience of the user of this software. This could be a human user, but could also be another piece of software that interacts with this project via an API of some sort. Example questions to ask as a user might include:

  • Is the UI or API sensible, predictable, usable?
  • Is the operation of the software appropriately observable (via logging, metrics, telemetry, and so forth)?
  • Does the proposed code change introduce breaking changes to the existing experience of the user?
  • Does the proposed code change follow the principle of least surprise?
  • Is the code change clearly and correctly documented at all appropriate levels?

Hat of the Developer

This hat is what many of us may think of first when we think of approaches to code review, and it absolutely is a useful and important one at that. This approach focuses heavily on the details of the code and implementation, asking questions like:

  • Can this code be understood and maintained by developers other than the original author?
  • Is it well-designed, useful, reusable, and appropriately abstracted and structured?
  • Does it avoid unnecessary complexity?
  • Does it avoid presenting multiple ways to achieve the same end result?
  • Is it DRY?
  • Does it include changes that aren’t actually needed at this time (YAGNI)?
  • Does it match the idioms and style of the existing code?
  • Is there a standard or “off the shelf” solution that could be used to solve this particular problem instead of writing new code?

Hat of the Tester

This hat is my personal favorite, as I started my career as a software tester. The tester’s goal is to think of what might go wrong, as well as what needs to go right. You might ask:

  • Does this change meet all appropriate requirements (explicitly stated as well as implicit ones)?
  • Is the logic correct, and is it testable to demonstrate correctness?
  • Are the provided tests correct, useful, and thorough?
  • Does the code expect (and appropriately handle) unexpected inputs, events, and data?

… there are two ways of constructing a software design: One way is to make it so simple that there are obviously no deficiencies, and the other way is to make it so complicated that there are no obvious deficiencies. – Sir Tony Hoare

Hat of the Attacker

This is another fun hat (or maybe I just like finding problems?). The attacker’s hat is closely related to the tester’s hat, but takes on a more devious frame of mind. Here you should ask questions like:

  • Does the code show adequate performance under usual (and unusual) conditions?
  • Can I make the code crash or throw an exception?
  • Can I make the code access something I shouldn’t be able to?
  • Can I make the software break, corrupt or delete data, or otherwise do the unexpected?

Hat of the Maintainer

This is the big-picture counterpart to the detail-focused developer’s hat. Here you should be asking questions like:

  • If certain issues or nitpicks keep coming up in review after review, is there something we should automate in the future?
  • Does the code change fit into the broader context of the project?
  • Are any new dependencies being added to the project? And if so, are they acceptable (actively maintained, appropriately licensed, and so forth)?

References and Further Reading

Many others have also written about the art and science of effective code reviews. Here are a few that I found particularly useful:


Conclusion

I hope that you have learned something new from this overview of code reviewing approaches and best practices. While many of us may take more pleasure from writing our own code than reviewing that of others, it’s always worth remembering that a code review represents both a learning and a teaching opportunity for everyone involved, and that ultimately, the goal of every code review is to help make a better software project. I urge you to make code reviews a priority!

-Glenn



ntc img
ntc img

Contact Us to Learn More

Share details about yourself & someone from our team will reach out to you ASAP!

Agile Planning Horizons (And how to Select Them) – An Enterprise Guide – Part 4

Blog Detail

Following from Parts 2 and 3 of this series which provided an overview of the Flow and Sprinting planning horizons, this article provides a closer look at the Program Increment, a Scaled Agile Planning Horizon.

Overview

Program Increment is a timebox artifact of SCALED AGILE operations. PI represents a committed Enterprise or Portfolio vision and execution plan. A PI is timebox for delivery of a Value Stream which is executed by “teams of teams” which are organized as Release Trains or Solution Trains.

  • Release Train – A long-lived team of Agile teams which delivers solutions in a value stream. The Release Train operates in synchronous cadence and practices regular, organized cross-communication around blocks and dependencies
  • Solution Train – An organized group of release trains that is focused on building large and complex solutions under a shared, over-arching business/technology mission.
Table of Planning Horizons

The Program Increment (PI) itself is a series of sprints (usually 4-6 sprints) in which multiple teams operate on a shared sprint cadence to deliver on one or more committed objectives of the PI.

The PI framework is an Enterprise-level synchronization of Agile Teams to deliver on large-scale enterprise objectives. Naturally, this requires large-scale synchronization of cadence and planning ceremonies across all teams. This includes daily or weekly syncs as needed (i.e. Scrum of Scrums and/or PO sync). This isn’t a status report. Instead, it is about communicating and coordinating about blockers/impediments, dependencies and “sequence of event” needs in order to ensure results are accomplished within the PI timebox for ALL teams in the train.

By synchronizing the PI planning and the following sprints across all Teams and Trains tasked with delivery, cross-domain planning is enabled. As a result, the Scaled Agile organization benefits from:

  • Structured Cadence
    • Improved predictability
    • Improved stakeholder confidence in wait times for committed capability
    • Support for regular planning and cross-functional coordination
    • Limitation of batch sizes of work to discrete intervals
    • Scheduled integration points
  • Synchronized Cross-Domain Planning
    • Improved control of injection of new work
    • Improved and routine dependency management
    • Supports full-system integration and assessment
    • Multiple feedback perspectives
    • Facilitates the ability for all stakeholders to engage with each other at the same time
    • Requirements and design planning occurs in a comprehensive way
    • Important stakeholder decisions are accelerated
    • Trains and Teams create and take responsibility for the PI plans created
    • Sequences and dependencies are understood and accounted for among Trains and Teams

The PI Planning Horizon requires a focus on dependencies and sequence of execution across teams for the purpose of achieving aligned Enterprise goals. A more detailed overview of the Program Increment can be found here: [https://www.scaledagileframework.com/program-increment/]

Ceremonies

There are a number of ceremonies which typically occur within the PI:

PI Planning – PI Planning is a Scaled Agile ceremony (usually lasting 1-3 days), in which multiple teams gather to establish, plan, and commit to the objectives to be delivered collectively by the teams. Each team will create and commit to its own objectives, but these will be in alignment with an over-arching vision and roadmap. The ceremony should have a standard agenda which starts with a presentation of the business context and vision, followed by team breakouts where each team’s plans and commitments are established and presented. A more detailed overview of this ceremony can be found here: [https://www.scaledagileframework.com/pi-planning/]]

Scrum of Scrums and PO Sync – Teams should have a built-in schedule to sync every 1 or 2 weeks (or more frequently if needed) to coordinate around various team or train dependencies and review progress and impediments to make plan adjustments as needed. This sync can occur as either a Scrum of Scrums or a PO Sync, or it can be combined into a single ceremony, as appropriate.

System Demo – The system demo is held collectively at the end of each sprint among all Teams in a Train. The objective is to demonstrate the current state of the solution overall, and gather feedback from sponsors/stakeholders/customers at the “Train Level”. This ceremony should be an integrated demonstration that provides an objective measure of functionality, progress, and ultimately value of the work delivered so far within the PI. This feedback provides an opportunity for the Train to validate its progress against PI objectives and/or to make course corrections. At the end of the last sprint in the PI, there is a “Final System Demo” which is delivered in the same way, and should be a fully integrated demonstration of the functional product achieved in the PI against the committed objectives. A more detailed review of this ceremony can be found here: [https://www.scaledagileframework.com/system-demo/]

Inspect and Adapt Workshop – The Inspect and Adapt Workshop is held at the end of the PI. It should include all of the teams and stakeholders who participated in PI Planning and/or were involved in supporting the execution of the PI. This event allows the teams and stakeholders to inspect the end result of the PI and engage in collective exercises for the purpose of continuous learning and improvement. The workshop consists of three parts:

  1. Final System Demo
  2. Quantitative and Qualitative Assessment
  3. Retrospective and Problem-Solving Workshop

A more detailed review of this ceremony can be found here: [https://www.scaledagileframework.com/inspect-and-adapt/]

Artifacts

Capability (or “Solution Capability”) – A Capability is a higher-level behavior or functionality that is identified, planned, refined, and committed for delivery by the Solution Train; typically the Solution Capability is delivered by multiple Release Trains. The Capability is composed of one or more Features and is sized to fit within the PI. Capabilities are usually created by Product Managers in collaboration with Product Owners and other key stakeholders. Capabilities are structured like features, but they are at a higher level of abstraction, describing system-level behavior and function in support of the definition and development of large solutions. Solution Capabilities comprise Strategic Solutions: the products, services and systems delivered to the customer.

Master Feature – In some Scaled Agile organizations or contexts, Master Features exist to help enterprises organize and group related Features. Master Features have the same properties as Features and Capabilities (overview, statement of benefit, and acceptance criteria) but exist at a level of abstraction higher than that of a Feature but lower than that of a Capability.

Feature – A Feature is a service or functionality that fulfills a stakeholder need.

The Feature is the primary artifact, which is identified, estimated, and groomed to “ready state”, and ultimately planned and committed to by the Release Train within the sprints of a Program Increment.

The feature should include a description of the service or function as well as a statement of benefit and acceptance criteria. The Feature is composed of one or more User Stories. Features are typically created by the Product Owner, with Solution (or System) Architects commonly creating enabler features that will be required for delivery of the functional Features and Capabilities. Features are committed to by Release Trains at the PI level, and the constituent Stories are committed to by Teams at the Sprint level.

Additional information about Features and Capabilities can be found here: https://www.scaledagileframework.com/features-and-capabilities/

Roles

Product Manager – The Product Manager is responsible for the Program backlog and roadmap, which is the definitive repository required to meet the upcoming release objectives, support strategic initiatives at the Program level, and provide the overall product vision and roadmap. The Product Manager works with Product Owners to ensure that Features being developed deliver the value and meet the needs of customers and stakeholders based on the overall PI objectives.

Release Train Engineer (RTE) – The RTE is a Scrum Master of Scrum Masters, acting at the Program level to coordinate resources, actions, planning, execution against commitments, removal of impediments, and ultimately results of multiple Scrum Teams at the Program level.

Solution Train Engineer (STE) – The STE is a Scrum Master of RTEs, acting at the Portfolio level.

Product Manager/Director – Senior product leader who understands the big picture and manages multiple Product Owners.

  • Owns and manages the delivery Roadmap on a timescale of 2 or more Quarters ahead.
  • Owns leadership sponsorship when newfound knowledge requires a pivot of scope or initiative, and manages the governance required.
  • Engages senior leadership to establish and hold to commitments made.
  • Communicates upstream, while maintaining situational awareness of tactical work in flight.

Solution Architect – Operates at senior management or executive level – ensures that the Product Manager(s) remain aligned with strategic objectives and empowers them to fulfill commitments.

  • Owns the Strategic Roadmap, commitments, and delivery at the Portfolio level.
  • Maintains situational awareness of Solution Trains in flight.
  • Resolves organizational blocks – works with the Enterprise to surface and resolve impediments before they block workflow.

Enterprise Architect – The Enterprise Architect is responsible for the optimization of manual and automated processes into an integrated environment. They act as lead architects by directing, integrating, and facilitating the work of Solution Architects while working on the target and transition architecture.

Executive Sponsor – Leads or sponsors Portfolio-level planning, sets priorities and objectives at the Enterprise and/or Portfolio level. Confirms scope, priority, and value delivery objectives and supports teams to deliver on them.

Pros and Cons

Pros

  • Well-led and capable organizations can deliver “market-wide” and “market-making” disruptive capability at rapid pace, and in a manner that can be sustained and iteratively enhanced by the enterprise.
  • Establish an “Enterprise-wide” culture of sustainable achievement, technical excellence, and continuous improvement.
  • Leverage institutional deep knowledge and human capital to rapidly introduce innovative and competitive capabilities, and keep the best assets of the enterprise engaged and delivering great value over a long period of time.
  • Provides a framework for removing non-performers at the enterprise level.
  • Provides a framework for promoting and enabling high performers at the enterprise level.

Cons

  • Requires a VERY high level of leadership, organizational communication, and collaboration capability – generally across multiple “traditional” silos.
  • PI Planning is a resource-intensive exercise that requires Executive-level commitment, communication/coordination, and follow-through. It requires not only a great effort for all teams, but it also requires executives to “get their hands dirty”, which tends to be one of the biggest challenges.
  • All of the requirements of the Sprinting framework, PLUS:
    • Executive-level understanding and leadership of Agile practices at a “hands-on” level. Talking about this topic is usually insufficient – Executive Champions will need to get involved with their vision and support, and live up to the responsibilities they ask of their teams.
    • Leadership-level accountability, honesty, and open communications among Enterprise peers is REQUIRED. (This is a big ask for traditional leadership who are focused on controlling their own silos.)
    • Enterprise-level ability to commit and dedicate the needed resources without behaving as a “Helicopter Manager”.

Entry Criteria

The entry criteria for an organization to adopt and implement Program Increment Planning Horizon is high. The organization should have multiple teams with experience and some level of maturity operating Scrum-based Agile development. Organizations should have strong competence with the Agile roles in addition to a clear Executive Vision for what is to be delivered, including:

  • Fully engaged Executive Product Leadership who is accountable for the performance and roadmap of the entire Product organization
  • Teams with the ability to operate Scrum (2-6 teams with similar performance and capabilities should be required)
  • ScrumMasters should be capable, fully committed, and engaged in their roles
  • Product Owners should be capable, fully committed, and engaged in their roles
  • Dev Teams must be capable, committed, and engaged in their roles

Conclusion

Team MUST be committed and focused on the PI objectives they’re responsible for. Teams that are “part time” or responsible for a variety of “Non-PI” deliverables struggle to perform in the PI Planning Horizon. Stakeholders must be engaged and committed to the teams they support. If stakeholders are not engaged and committed, the team will question the value of the work they’ve committed to. Stakeholders should be accountable to the Agile Delivery Process, and they should be available and engaged as needed based on team requirements.

Thank you for reading this series!

-Matt



ntc img
ntc img

Contact Us to Learn More

Share details about yourself & someone from our team will reach out to you ASAP!

Agile Planning Horizons (and How to Choose Them) – An Enterprise Guide – Part 3

Blog Detail

Following from Parts 1 and 2 of this series, which provided a high-level overview of three common Agile Planning Horizons, this article provides a closer look at the Sprinting Planning Horizon.

Overview – Sprinting Planning Horizon

Sprinting is an Agile Planning Horizon used by Scrum teams to rapidly and iteratively deliver products of the highest possible value within timeboxes called Sprints. Sprinting is a fixed cadence of iterative cycles (typically two weeks but not shorter than 1 week and not more than 4 weeks).  The short delivery cycles and collaborative nature of Sprinting makes this framework ideal for projects with rapidly changing, highly emergent requirements.

Roles in the Sprinting Framework

There are three primary roles in the Sprinting / Scrum framework:

  • Product Owner (PO) – The primary job of the product owner is to be the customer representative (the voice of the customer), turning the vision into a workable product backlog for the team to execute and managing the return on investment/business value of the product or service under development. The product owner brings clarity to the vision of the product to be delivered and works with customers and stakeholders to build the list of requirements that makes up the product backlog. The PO owns the product backlog, sets priority of the work, defines requirements and acceptance criteria, and ultimately accepts or rejects the work delivered.
  • ScrumMaster (SM) – The role of the ScumMaster is multi-faceted. The SM works to build and maintain a high-performing team, manages the various scrum ceremonies, removes blocks and impediments, keeps the team focused and on track, and keeps a view of the big picture while looking for places where the team can make improvements. The SM also has a role of protecting the team’s collective health and protects the team’s “psychological security” to operate properly.
  • Scrum Team Members – The team members deliver on the product vision and build the functionality needed to implement features from the prioritized product backlog. The Scrum Team should range between 3 to 10 members, and the collective team will possess all of the skills and capabilities required to deliver on the product vision. The Scrum Team should be composed of “T-shaped” individuals – which is to say that, while they may specialize in a particular technical area, they have a broad range of technical capabilities. The Scrum Team owns grooming & refinement of the backlog to get stories to a “ready to work” state. The Scrum Team also owns the “Sprint Commitment” – which is the team’s commitment for what work will be delivered in a given Sprint. They own end-to-end development, testing, documentation, and delivery of valuable product functionality.

Ceremonies

In the Sprinting framework, teams conduct numerous ceremonies to plan work, deliver work, and inspect & adapt for continuous improvement.

  • Sprint Planning – During the planning meeting, the team reviews the stories, estimates against team capacity and priority, and commits to the delivery of this body of work within the Sprint timebox. 
  • Daily Standup – During the Sprint, the team participates in a Daily Standup to review work done the day prior, coordinate the work to be delivered that day, and identify any impediments/blocks to be resolved on that day. 
  • Backlog Grooming/Refinement – Also during the Sprint, the team holds a number of backlog grooming & refinement meetings to prioritize and estimate the work that will be needed for subsequent Sprints. 
  • Retrospective – At the end of the Sprint, the team reviews the work delivered vs what they committed to in Sprint planning.  They take stock of what went well and what did not.  They also identify initiatives for continued improvement, and when possible, they create stories for the next Sprint to account for taking that improvement action.
  • Sprint Demo – At the end of the Sprint, the team demonstrates the functionality completed to stakeholders and takes feedback for additional iterative changes/improvements for future Sprints.  These action items are then represented in the Product Backlog and prioritized for delivery in subsequent Sprints.

Artifacts

The work artifact for a Sprint is the Story.  Generally, one or more stories make up Features which represent a unit of functionality that delivers considerable business value and fulfills a customer need.  In the Sprint planning horizon, a team commits to and delivers stories which are needed by the Customer.  Delivery of features can span more than one Sprint, whereas delivery of stories should be contained within the Sprint that the story was committed to.

Typically, the stories will have more robust requirements, acceptance criteria, and task lists than a Kanban issue.  The Sprint stories will also include an estimated weight (in story points), and they will generally have some way to document the actual time spent for review and measurement in the Retrospective following the Sprint.

Stories on the sprint board will also generally identify which Feature/Epic they contribute to – they will identify the initiative or sprint goal that each story is aligned to.

The Sprint Board

Generally, the sprint board is similar to a Kanban board – the main difference is that it represents a specific timebox with predetermined start and end dates.  There can be more “status” columns or labels for the stories on the board, including identification of blocked work, work that has upstream/downstream dependencies, or work that is a “stretch goal” for a sprint.

Pros and Cons

Pros

Here are the conditions which lend themselves to a Sprinting-based planning horizon. The Sprinting framework is ideal for:

  • Complicated or complex projects where the requirements or technology (or both) are uncertain; stakeholders may not be certain about what technology will be best for achieving the desired value, or what the final product will look like (but they know what they need).
  • The priority and requirements of the work to be delivered are understood and accepted at a general and broad-enough level that the work can be planned and delivered within 2 – 8 weeks. This is to say that the business is “certain enough” about the product needs & requirements that the scope can be “locked” and committed for delivery within the sprint “timebox”; further, the delivered capability will be useful and valuable upon delivery.
  • Situations where the Team needs to collaborate with customers/stakeholders to discover and identify the detailed needs/requirements, and where flexibility and negotiation can happen in the delivery process.
Sprinting Complicated to Complex

Source: [“The Scrum Field Guide” by Mitch Lacey]

Cons

Here are the conditions which DO NOT lend themselves to a Sprinting-based planning horizon:

  • The organization cannot commit DEDICATED resources to the effort for all three Scrum roles (PO, SM, Team).
  • Delivery requirements are unstable and change continuously.
  • Stakeholders and/or customers are not consistently available to engage, collaborate, or provide critical feedback. Stakeholders and/or customers are unable or unwilling to actively participate in the Sprint process.
  • Roles lacking authority – individuals are designated to conduct a Scrum role without the authority to make concrete decisions for the team or organization they represent.
  • The inability to contain or control “Helicopter Managers” – Managers who undermine the Sprint framework by asking individual team members for work or support which is not sanctioned within the Team’s Sprint Commitment – and the lack of authority on the part of the SM, PO or Team Member to stop this behavior.

Conclusion

The entry criteria for teams to adopt a Sprint-based agile planning horizon is significant. The organization MUST be able to identify the proper resources for the Product Owner, Scrum Master, and Scrum Team roles. The Team members must own and be accountable to operating within the framework for both themselves and their Teams.

Further, the organization MUST dedicate those resources to achievement of the required objectives.

The organization MUST commit to upholding the standards and duties of the roles and team commitment. This includes:

  • Preserve the sanctity of defined Scrum roles
  • Commitment of dedicated capacity of the Scrum Team – 80%+ of all time for each member
  • Commitment to uphold and exercise Scrum Ceremonies
    • Sprint Planning
    • Daily Standups
    • Backlog Grooming / Refinement
    • Sprint Retrospectives
    • Sprint Reviews / Sprint Demonstrations
  • Teams must be able to plan work in concert with Product Ownership and commit to the work within each sprint
  • Stakeholders should have a vested interest in the work being delivered, and make themselves available for feedback and collaboration as needed by the Scrum team

Thank you! The final post in this series is a closer look at the Program Increment Agile Planning Horizon.

-Matt



ntc img
ntc img

Contact Us to Learn More

Share details about yourself & someone from our team will reach out to you ASAP!