Język

Language not available Language not available Language not available

Ten post nie jest dostępny w Twoim języku. Oto kilka innych opcji:

Guiding a monolith with a gentle touch: pairing codeowners and lint rules

Sep 7 2021 - By Noah Manneschmidt

My name is Noah Manneschmidt and I’ve been a front-end developer at Twitch for 5 years now. I’m currently a member of the Browser Clients team, and our mission is to keep the other front-end developers at Twitch as productive as possible, while ensuring that Twitch viewers are able to load our website and live videos quickly. 

The majority of our work is from our stewardship of Twitch’s main front-end codebase: also known as Twilight. On any given week, it hosts nearly 100 developers from dozens of different teams, all working in parallel on millions of lines of Typescript. It includes the code for the majority of our main website, all of the creator dashboard, the Twitch Studio broadcaster app, and much more. Twilight has been in production for about four years now, and keeping code quality high, especially as best practices changed over the years, quickly became a challenge. Fortunately, we’ve built a few nifty tools that have enabled my relatively small team of ten developers to distribute the maintenance effort among all project contributors. Over time, it has coordinated long term code refactors across thousands of files, something we simply didn’t have the resources to do on our own.

Establishing Ownership

One of the earliest problems we faced as the project grew was ownership. As stewards of the platform, my team would often need to make changes to code written by other teams. A simple (but flawed) model of ownership we already had was an implicit one, found in the commit history of each file in the project. However, auditing the git commit history of a file to find recent or significant individual contributors to a file can be a difficult task. What we really needed was an explicit model, where we designate ownership of individual files and folders within the codebase to specific teams. This allows us to hold the entire team accountable for the code quality under their ownership, and avoids problems that arise when the ownership is centered on individuals. For example, people often change teams, leave the company, or simply go on vacation, and can’t answer questions about a particular bit of complex logic.

One existing solution that fit our needs was Github’s CODEOWNERS file. By adding a plain text file to our repo and making a few clicks inside the branch protection settings, we could enforce that owners of any code changed in a Pull Request (PR) had to review it before it could merge.

Using this feature comes with some important notes about the way Github interacts with the CODEOWNERS file. First, rule ordering within the file is critically important. And second, the fact that:

If any line in your CODEOWNERS file contains invalid syntax, the file will not be detected and will not be used to request reviews.

Given the large size of the project, our initial CODEOWNERS file was already several hundred lines long and growing quickly. To keep it organized, we set up a simple script in CI to enforce some basic sanity checks, such as keeping file path rules alphabetized. We also created a special team, @twilight/unowned, which is given ownership of everything by default using a single wildcard rule at the start of the file. With this, any PR introducing new files not already covered by a more specific rule in the CODEOWNERS file will be clearly flagged, and we can request that others update our ownership paths to cover their new additions. We also took the concept of the CODEOWNERS file one step further by extending the file format, adding a simple table of useful contact info for each team to the top of the repo.

# Team Contact Info. Double pound prefix makes this section parsable by `twitch-codeowners` library.
## team slack-channel engineering-manager jira-project-key
## @twilight/video #video @alice VID
## @twilight/chat #chat-engineering @bob CHAT
## @twilight/browser-clients #twilight @chris BROWSER
# ...etc

# You don't want to hit this. Own your code!
* @twilight/unowned

# Source ownership. List must be alphabetical.
/src/common/ @twilight/browser-clients
/src/features/chat/ @twilight/chat
/src/features/video-player/ @twilight/video
# ...etc

We have a few internal tools that parse this extra info and provide easy access to it in places where it makes sense, such as inside Visual Studio Code via a custom plugin.

Failure Snapshots

Having all the ownership info consolidated as a single source of truth was a huge step forward for the project. But, there were some migration efforts that couldn’t be handled by our team alone. For example, we wanted to migrate away from deprecated React lifecycle methods or update our API requests from old endpoints to our modern GraphQL backend. While neither case called for urgent attention, we didn’t want Twilight developers to keep adding more tech debt to the project by continuing to use these older patterns.

Our first line of defense against patterns we want to discourage is usually a lint rule. We either find existing lint rule plugins or write a custom one for the pattern at hand, and apply it at either “error” or “warning” level. Even as an error, these still amount to mere suggestions, as developers can choose to ignore them with the addition of a single comment. We still needed more control over the project to maintain quality.

For a simple issue, we could include an auto-fix with the lint rule and apply that to the entire project, but these issues were complex, requiring the attention of a developer to address each one. Applying these new lint rules as errors would require us to add hundreds of lint ignore comments at each existing site, so we initially kept them as warnings and included custom messages with the warning pointing developers to more complete documentation.

In order to keep any further warnings from being added for these specific lint rules, we set up a new pattern in the project that we internally call “mapped lint rules” but would be better referred to as “lint failure snapshots”. We save a snapshot to disk of all the current known uses of deprecated patterns, allowing existing ones to exist without errors, but raising red flags for any new additions.

To start, we first split our lint configuration file into smaller pieces:

The lint plugin in a developer’s editor will load the final config and display everything at once, as will our typical CI lint check. In addition, we added a new, separate script modeled off of Jest’s snapshot testing that runs in one of two modes: check or update.

In update mode (adding -u) the script first invokes eslint using the mapped config, and critically, it configures allowInlineConfig to false, preventing the use of inline disable comments. The results of the lint pass are then split out per rule and file and written to disk, one JSON “failure snapshot file” per lint rule. The contents of each file is a single object where each key is the path to a file with failures and the value is a count of how many failures a given rule produced within that file.

{
  "/src/features/twitch-prime/components/prime-offers.tsx": 1,
  "/src/features/video-player/components/player-shell.tsx": 1,
  "/src/pages/video-tools/components/highlighter-pins.tsx": 1,
  "/src/sites/clips/features/chat-replay/components/chat-card.tsx": 1
}

The results are committed to source control in a location under the Browser Clients team’s ownership. In CI, the script runs in check mode, comparing the number of failures in code to the number written out to disk, and fails if the number of failures from the latest code are higher than what is recorded in the snapshot files.

Power in the Pairing 

Put into practice, this combination of explicit ownership and lint failure snapshots has given us very nuanced control over how and when specific code patterns are used in the project. Developers are proactively warned against using deprecated things we want to move away from, but if they have a compelling reason, they can update the lint snapshot file which then requires a review from the Browser Clients team (thanks to the CODEOWNERS rule). In the PR, we can discuss with the developer the tradeoffs of using the pattern in question and find the best solution together. As a result, these updates have allowed us to drive a wide variety of long-term efforts to meet security standards, improve accessibility on the site, keep Twilight organized, and improve site performance for Twitch viewers!

That said, lint rules can’t solve every problem, so we’re still hard at work keeping the project running smoothly, and finding new ways to improve our processes. Next up: keeping our build, test, and lint times in check as Twilight approaches 3 million lines of code.


We’re building the future of live entertainment, and we’d do it even better with you. Head to our career site to learn more about what it is like to work at Twitch and how you can join our squad

W innych wiadomościach