222 lines
24 KiB
HTML
222 lines
24 KiB
HTML
|
<!DOCTYPE HTML>
|
||
|
|
<html lang="en" class="sidebar-visible no-js light">
|
||
|
|
<head>
|
||
|
|
<!-- Book generated using mdBook -->
|
||
|
|
<meta charset="UTF-8">
|
||
|
|
<title>Reviewing Code - Synapse</title>
|
||
|
|
<!-- Custom HTML head -->
|
||
|
|
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
|
||
|
|
<meta name="description" content="">
|
||
|
|
<meta name="viewport" content="width=device-width, initial-scale=1">
|
||
|
|
<meta name="theme-color" content="#ffffff" />
|
||
|
|
|
||
|
|
<link rel="icon" href="../favicon.svg">
|
||
|
|
<link rel="shortcut icon" href="../favicon.png">
|
||
|
|
<link rel="stylesheet" href="../css/variables.css">
|
||
|
|
<link rel="stylesheet" href="../css/general.css">
|
||
|
|
<link rel="stylesheet" href="../css/chrome.css">
|
||
|
|
<link rel="stylesheet" href="../css/print.css" media="print">
|
||
|
|
<!-- Fonts -->
|
||
|
|
<link rel="stylesheet" href="../FontAwesome/css/font-awesome.css">
|
||
|
|
<link rel="stylesheet" href="../fonts/fonts.css">
|
||
|
|
<!-- Highlight.js Stylesheets -->
|
||
|
|
<link rel="stylesheet" href="../highlight.css">
|
||
|
|
<link rel="stylesheet" href="../tomorrow-night.css">
|
||
|
|
<link rel="stylesheet" href="../ayu-highlight.css">
|
||
|
|
|
||
|
|
<!-- Custom theme stylesheets -->
|
||
|
|
<link rel="stylesheet" href="../docs/website_files/table-of-contents.css">
|
||
|
|
<link rel="stylesheet" href="../docs/website_files/remove-nav-buttons.css">
|
||
|
|
<link rel="stylesheet" href="../docs/website_files/indent-section-headers.css">
|
||
|
|
</head>
|
||
|
|
<body>
|
||
|
|
<!-- Provide site root to javascript -->
|
||
|
|
<script type="text/javascript">
|
||
|
|
var path_to_root = "../";
|
||
|
|
var default_theme = window.matchMedia("(prefers-color-scheme: dark)").matches ? "navy" : "light";
|
||
|
|
</script>
|
||
|
|
|
||
|
|
<!-- Work around some values being stored in localStorage wrapped in quotes -->
|
||
|
|
<script type="text/javascript">
|
||
|
|
try {
|
||
|
|
var theme = localStorage.getItem('mdbook-theme');
|
||
|
|
var sidebar = localStorage.getItem('mdbook-sidebar');
|
||
|
|
if (theme.startsWith('"') && theme.endsWith('"')) {
|
||
|
|
localStorage.setItem('mdbook-theme', theme.slice(1, theme.length - 1));
|
||
|
|
}
|
||
|
|
if (sidebar.startsWith('"') && sidebar.endsWith('"')) {
|
||
|
|
localStorage.setItem('mdbook-sidebar', sidebar.slice(1, sidebar.length - 1));
|
||
|
|
}
|
||
|
|
} catch (e) { }
|
||
|
|
</script>
|
||
|
|
|
||
|
|
<!-- Set the theme before any content is loaded, prevents flash -->
|
||
|
|
<script type="text/javascript">
|
||
|
|
var theme;
|
||
|
|
try { theme = localStorage.getItem('mdbook-theme'); } catch(e) { }
|
||
|
|
if (theme === null || theme === undefined) { theme = default_theme; }
|
||
|
|
var html = document.querySelector('html');
|
||
|
|
html.classList.remove('no-js')
|
||
|
|
html.classList.remove('light')
|
||
|
|
html.classList.add(theme);
|
||
|
|
html.classList.add('js');
|
||
|
|
</script>
|
||
|
|
|
||
|
|
<!-- Hide / unhide sidebar before it is displayed -->
|
||
|
|
<script type="text/javascript">
|
||
|
|
var html = document.querySelector('html');
|
||
|
|
var sidebar = 'hidden';
|
||
|
|
if (document.body.clientWidth >= 1080) {
|
||
|
|
try { sidebar = localStorage.getItem('mdbook-sidebar'); } catch(e) { }
|
||
|
|
sidebar = sidebar || 'visible';
|
||
|
|
}
|
||
|
|
html.classList.remove('sidebar-visible');
|
||
|
|
html.classList.add("sidebar-" + sidebar);
|
||
|
|
</script>
|
||
|
|
|
||
|
|
<nav id="sidebar" class="sidebar" aria-label="Table of contents">
|
||
|
|
<div class="sidebar-scrollbox">
|
||
|
|
<ol class="chapter"><li class="chapter-item expanded affix "><li class="part-title">Introduction</li><li class="chapter-item expanded "><a href="../welcome_and_overview.html">Welcome and Overview</a></li><li class="chapter-item expanded affix "><li class="part-title">Setup</li><li class="chapter-item expanded "><a href="../setup/installation.html">Installation</a></li><li class="chapter-item expanded "><a href="../postgres.html">Using Postgres</a></li><li class="chapter-item expanded "><a href="../reverse_proxy.html">Configuring a Reverse Proxy</a></li><li class="chapter-item expanded "><a href="../setup/forward_proxy.html">Configuring a Forward/Outbound Proxy</a></li><li class="chapter-item expanded "><a href="../turn-howto.html">Configuring a Turn Server</a></li><li><ol class="section"><li class="chapter-item expanded "><a href="../setup/turn/coturn.html">coturn TURN server</a></li><li class="chapter-item expanded "><a href="../setup/turn/eturnal.html">eturnal TURN server</a></li></ol></li><li class="chapter-item expanded "><a href="../delegate.html">Delegation</a></li><li class="chapter-item expanded affix "><li class="part-title">Upgrading</li><li class="chapter-item expanded "><a href="../upgrade.html">Upgrading between Synapse Versions</a></li><li class="chapter-item expanded affix "><li class="part-title">Usage</li><li class="chapter-item expanded "><a href="../federate.html">Federation</a></li><li class="chapter-item expanded "><a href="../usage/configuration/index.html">Configuration</a></li><li><ol class="section"><li class="chapter-item expanded "><a href="../usage/configuration/config_documentation.html">Configuration Manual</a></li><li class="chapter-item expanded "><a href="../usage/configuration/homeserver_sample_config.html">Homeserver Sample Config File</a></li><li class="chapter-item expanded "><a href="../usage/configuration/logging_sample_config.html">Logging Sample Config File</a></li><li class="chapter-item expanded "><a href="../structured_logging.html">Structured Logging</a></li><li class="chapter-item expanded "><a href="../templates.html">Templates</a></li><li class="chapter-item expanded "><a href="../usage/configuration/user_authentication/index.html">User Authentication</a></li><li><ol class="section"><li class="chapter-item expanded "><a href="../usage/configuration/user_authentication/single_sign_on/index.html">Single-Sign On</a></li><li><ol class="section"><li class="chapter-item expanded "><a href="../openid.html">OpenID Connect</a></li><li class="chapter-item expanded "><a href="../usage/configuration/user_authentication/single_sign_on/saml.html">SAML</a></li><li class="chapter-item expanded "><a href="../usage/configuration/user_authentication/single_sign_on/cas.html">CAS</a></li><li class="chapter-item expanded "><a href="../sso_mapping_providers.html">SSO Mapping Providers</a></li></ol></li><li class="chapter-item expanded "><a href="../password_auth_providers.html">Password Auth Providers</a></li><li class="chapter-item expanded "><a href="../jwt.html">JSON Web Tokens</a></li><li class="chapter-item expanded "><a href="../usage/configuration/user_authentication/refresh_tokens.html">Refresh Tokens</a></li></ol></li><li class="chapter-item expanded "><a href="../CAPTCHA_SETUP.html">Registration Captcha</a></li><li class="chapter-item expanded "><a href="../application_services.html">Application Services</a></li><li class="chapter-item expanded "><a href="../server_notices.html">Server Notices</a></li><li class="chapter-item expanded "><a href="../consent_tracking.html">Consent Tracking</a></li><li class="chapter-item expanded "><a href="../user_directory.html">User Directory</a></li><li class="chapter-item expanded "><a href="../message_retention_policies.html">Message Retention Policies</a></li><li class="chapter-item expanded "><a href="../modules/index.html">Pluggable Modules</a></li><li><ol class="section"><li class="chapter-item expanded "><a href="../modules/writing_a_module.html">Writing a module</a></li><li><ol class="section"><li class="chapter-item expanded "
|
||
|
|
</div>
|
||
|
|
<div id="sidebar-resize-handle" class="sidebar-resize-handle"></div>
|
||
|
|
</nav>
|
||
|
|
|
||
|
|
<div id="page-wrapper" class="page-wrapper">
|
||
|
|
|
||
|
|
<div class="page">
|
||
|
|
<div id="menu-bar-hover-placeholder"></div>
|
||
|
|
<div id="menu-bar" class="menu-bar sticky bordered">
|
||
|
|
<div class="left-buttons">
|
||
|
|
<button id="sidebar-toggle" class="icon-button" type="button" title="Toggle Table of Contents" aria-label="Toggle Table of Contents" aria-controls="sidebar">
|
||
|
|
<i class="fa fa-bars"></i>
|
||
|
|
</button>
|
||
|
|
<button id="theme-toggle" class="icon-button" type="button" title="Change theme" aria-label="Change theme" aria-haspopup="true" aria-expanded="false" aria-controls="theme-list">
|
||
|
|
<i class="fa fa-paint-brush"></i>
|
||
|
|
</button>
|
||
|
|
<ul id="theme-list" class="theme-popup" aria-label="Themes" role="menu">
|
||
|
|
<li role="none"><button role="menuitem" class="theme" id="light">Light (default)</button></li>
|
||
|
|
<li role="none"><button role="menuitem" class="theme" id="rust">Rust</button></li>
|
||
|
|
<li role="none"><button role="menuitem" class="theme" id="coal">Coal</button></li>
|
||
|
|
<li role="none"><button role="menuitem" class="theme" id="navy">Navy</button></li>
|
||
|
|
<li role="none"><button role="menuitem" class="theme" id="ayu">Ayu</button></li>
|
||
|
|
</ul>
|
||
|
|
<button id="search-toggle" class="icon-button" type="button" title="Search. (Shortkey: s)" aria-label="Toggle Searchbar" aria-expanded="false" aria-keyshortcuts="S" aria-controls="searchbar">
|
||
|
|
<i class="fa fa-search"></i>
|
||
|
|
</button>
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<h1 class="menu-title">Synapse</h1>
|
||
|
|
|
||
|
|
<div class="right-buttons">
|
||
|
|
<a href="../print.html" title="Print this book" aria-label="Print this book">
|
||
|
|
<i id="print-button" class="fa fa-print"></i>
|
||
|
|
</a>
|
||
|
|
<a href="https://github.com/matrix-org/synapse" title="Git repository" aria-label="Git repository">
|
||
|
|
<i id="git-repository-button" class="fa fa-github"></i>
|
||
|
|
</a>
|
||
|
|
<a href="https://github.com/matrix-org/synapse/edit/develop/docs/development/reviews.md" title="Suggest an edit" aria-label="Suggest an edit">
|
||
|
|
<i id="git-edit-button" class="fa fa-edit"></i>
|
||
|
|
</a>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<div id="search-wrapper" class="hidden">
|
||
|
|
<form id="searchbar-outer" class="searchbar-outer">
|
||
|
|
<input type="search" id="searchbar" name="searchbar" placeholder="Search this book ..." aria-controls="searchresults-outer" aria-describedby="searchresults-header">
|
||
|
|
</form>
|
||
|
|
<div id="searchresults-outer" class="searchresults-outer hidden">
|
||
|
|
<div id="searchresults-header" class="searchresults-header"></div>
|
||
|
|
<ul id="searchresults">
|
||
|
|
</ul>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
<!-- Apply ARIA attributes after the sidebar and the sidebar toggle button are added to the DOM -->
|
||
|
|
<script type="text/javascript">
|
||
|
|
document.getElementById('sidebar-toggle').setAttribute('aria-expanded', sidebar === 'visible');
|
||
|
|
document.getElementById('sidebar').setAttribute('aria-hidden', sidebar !== 'visible');
|
||
|
|
Array.from(document.querySelectorAll('#sidebar a')).forEach(function(link) {
|
||
|
|
link.setAttribute('tabIndex', sidebar === 'visible' ? 0 : -1);
|
||
|
|
});
|
||
|
|
</script>
|
||
|
|
|
||
|
|
<div id="content" class="content">
|
||
|
|
<main>
|
||
|
|
<!-- Page table of contents -->
|
||
|
|
<div class="sidetoc">
|
||
|
|
<nav class="pagetoc"></nav>
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<h1 id="some-notes-on-how-we-do-reviews"><a class="header" href="#some-notes-on-how-we-do-reviews">Some notes on how we do reviews</a></h1>
|
||
|
|
<p>The Synapse team works off a shared review queue -- any new pull requests for
|
||
|
|
Synapse (or related projects) has a review requested from the entire team. Team
|
||
|
|
members should process this queue using the following rules:</p>
|
||
|
|
<ul>
|
||
|
|
<li>Any high urgency pull requests (e.g. fixes for broken continuous integration
|
||
|
|
or fixes for release blockers);</li>
|
||
|
|
<li>Follow-up reviews for pull requests which have previously received reviews;</li>
|
||
|
|
<li>Any remaining pull requests.</li>
|
||
|
|
</ul>
|
||
|
|
<p>For the latter two categories above, older pull requests should be prioritised.</p>
|
||
|
|
<p>It is explicit that there is no priority given to pull requests from the team
|
||
|
|
(vs from the community). If a pull request requires a quick turn around, please
|
||
|
|
explicitly communicate this via <a href="https://matrix.to/#/#synapse-dev:matrix.org">#synapse-dev:matrix.org</a>
|
||
|
|
or as a comment on the pull request.</p>
|
||
|
|
<p>Once an initial review has been completed and the author has made additional changes,
|
||
|
|
follow-up reviews should go back to the same reviewer. This helps build a shared
|
||
|
|
context and conversation between author and reviewer.</p>
|
||
|
|
<p>As a team we aim to keep the number of inflight pull requests to a minimum to ensure
|
||
|
|
that ongoing work is finished before starting new work.</p>
|
||
|
|
<h2 id="performing-a-review"><a class="header" href="#performing-a-review">Performing a review</a></h2>
|
||
|
|
<p>To communicate to the rest of the team the status of each pull request, team
|
||
|
|
members should do the following:</p>
|
||
|
|
<ul>
|
||
|
|
<li>Assign themselves to the pull request (they should be left assigned to the
|
||
|
|
pull request until it is merged, closed, or are no longer the reviewer);</li>
|
||
|
|
<li>Review the pull request by leaving comments, questions, and suggestions;</li>
|
||
|
|
<li>Mark the pull request appropriately (as needing changes or accepted).</li>
|
||
|
|
</ul>
|
||
|
|
<p>If you are unsure about a particular part of the pull request (or are not confident
|
||
|
|
in your understanding of part of the code) then ask questions or request review
|
||
|
|
from the team again. When requesting review from the team be sure to leave a comment
|
||
|
|
with the rationale on why you're putting it back in the queue.</p>
|
||
|
|
|
||
|
|
</main>
|
||
|
|
|
||
|
|
<nav class="nav-wrapper" aria-label="Page navigation">
|
||
|
|
<!-- Mobile navigation buttons -->
|
||
|
|
<a rel="prev" href="../code_style.html" class="mobile-nav-chapters previous" title="Previous chapter" aria-label="Previous chapter" aria-keyshortcuts="Left">
|
||
|
|
<i class="fa fa-angle-left"></i>
|
||
|
|
</a>
|
||
|
|
<a rel="next" href="../development/releases.html" class="mobile-nav-chapters next" title="Next chapter" aria-label="Next chapter" aria-keyshortcuts="Right">
|
||
|
|
<i class="fa fa-angle-right"></i>
|
||
|
|
</a>
|
||
|
|
<div style="clear: both"></div>
|
||
|
|
</nav>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<nav class="nav-wide-wrapper" aria-label="Page navigation">
|
||
|
|
<a rel="prev" href="../code_style.html" class="nav-chapters previous" title="Previous chapter" aria-label="Previous chapter" aria-keyshortcuts="Left">
|
||
|
|
<i class="fa fa-angle-left"></i>
|
||
|
|
</a>
|
||
|
|
<a rel="next" href="../development/releases.html" class="nav-chapters next" title="Next chapter" aria-label="Next chapter" aria-keyshortcuts="Right">
|
||
|
|
<i class="fa fa-angle-right"></i>
|
||
|
|
</a>
|
||
|
|
</nav>
|
||
|
|
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<script type="text/javascript">
|
||
|
|
window.playground_copyable = true;
|
||
|
|
</script>
|
||
|
|
<script src="../elasticlunr.min.js" type="text/javascript" charset="utf-8"></script>
|
||
|
|
<script src="../mark.min.js" type="text/javascript" charset="utf-8"></script>
|
||
|
|
<script src="../searcher.js" type="text/javascript" charset="utf-8"></script>
|
||
|
|
<script src="../clipboard.min.js" type="text/javascript" charset="utf-8"></script>
|
||
|
|
<script src="../highlight.js" type="text/javascript" charset="utf-8"></script>
|
||
|
|
<script src="../book.js" type="text/javascript" charset="utf-8"></script>
|
||
|
|
|
||
|
|
<!-- Custom JS scripts -->
|
||
|
|
<script type="text/javascript" src="../docs/website_files/table-of-contents.js"></script>
|
||
|
|
</body>
|
||
|
|
</html>
|