diff options
Diffstat (limited to '')
| -rw-r--r-- | v1.75/development/reviews.html | 222 |
1 files changed, 222 insertions, 0 deletions
diff --git a/v1.75/development/reviews.html b/v1.75/development/reviews.html new file mode 100644 index 0000000000..661d0cd7d3 --- /dev/null +++ b/v1.75/development/reviews.html @@ -0,0 +1,222 @@ +<!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 "><a href="../modules/spam_checker_callbacks.html">Spam checker callbacks</a></li><li class="chapter-item expanded "><a href="../modules/third_party_rules_callbacks.html">Third-party rules callbacks</a></li><li class="chapter-item expanded "><a href="../modules/presence_router_callbacks.html">Presence router callbacks</a></li><li class="chapter-item expanded "><a href="../modules/account_validity_callbacks.html">Account validity callbacks</a></li><li class="chapter-item expanded "><a href="../modules/password_auth_provider_callbacks.html">Password auth provider callbacks</a></li><li class="chapter-item expanded "><a href="../modules/background_update_controller_callbacks.html">Background update controller callbacks</a></li><li class="chapter-item expanded "><a href="../modules/account_data_callbacks.html">Account data callbacks</a></li><li class="chapter-item expanded "><a href="../modules/porting_legacy_module.html">Porting a legacy module to the new interface</a></li></ol></li></ol></li><li class="chapter-item expanded "><a href="../workers.html">Workers</a></li><li><ol class="section"><li class="chapter-item expanded "><a href="../synctl_workers.html">Using synctl with Workers</a></li><li class="chapter-item expanded "><a href="../systemd-with-workers/index.html">Systemd</a></li></ol></li></ol></li><li class="chapter-item expanded "><a href="../usage/administration/index.html">Administration</a></li><li><ol class="section"><li class="chapter-item expanded "><a href="../usage/administration/admin_api/index.html">Admin API</a></li><li><ol class="section"><li class="chapter-item expanded "><a href="../admin_api/account_validity.html">Account Validity</a></li><li class="chapter-item expanded "><a href="../usage/administration/admin_api/background_updates.html">Background Updates</a></li><li class="chapter-item expanded "><a href="../admin_api/event_reports.html">Event Reports</a></li><li class="chapter-item expanded "><a href="../admin_api/media_admin_api.html">Media</a></li><li class="chapter-item expanded "><a href="../admin_api/purge_history_api.html">Purge History</a></li><li class="chapter-item expanded "><a href="../admin_api/register_api.html">Register Users</a></li><li class="chapter-item expanded "><a href="../usage/administration/admin_api/registration_tokens.html">Registration Tokens</a></li><li class="chapter-item expanded "><a href="../admin_api/room_membership.html">Manipulate Room Membership</a></li><li class="chapter-item expanded "><a href="../admin_api/rooms.html">Rooms</a></li><li class="chapter-item expanded "><a href="../admin_api/server_notices.html">Server Notices</a></li><li class="chapter-item expanded "><a href="../admin_api/statistics.html">Statistics</a></li><li class="chapter-item expanded "><a href="../admin_api/user_admin_api.html">Users</a></li><li class="chapter-item expanded "><a href="../admin_api/version_api.html">Server Version</a></li><li class="chapter-item expanded "><a href="../usage/administration/admin_api/federation.html">Federation</a></li></ol></li><li class="chapter-item expanded "><a href="../manhole.html">Manhole</a></li><li class="chapter-item expanded "><a href="../metrics-howto.html">Monitoring</a></li><li><ol class="section"><li class="chapter-item expanded "><a href="../usage/administration/monitoring/reporting_homeserver_usage_statistics.html">Reporting Homeserver Usage Statistics</a></li></ol></li><li class="chapter-item expanded "><a href="../usage/administration/monthly_active_users.html">Monthly Active Users</a></li><li class="chapter-item expanded "><a href="../usage/administration/understanding_synapse_through_grafana_graphs.html">Understanding Synapse Through Grafana Graphs</a></li><li class="chapter-item expanded "><a href="../usage/administration/useful_sql_for_admins.html">Useful SQL for Admins</a></li><li class="chapter-item expanded "><a href="../usage/administration/database_maintenance_tools.html">Database Maintenance Tools</a></li><li class="chapter-item expanded "><a href="../usage/administration/state_groups.html">State Groups</a></li><li class="chapter-item expanded "><a href="../usage/administration/request_log.html">Request log format</a></li><li class="chapter-item expanded "><a href="../usage/administration/admin_faq.html">Admin FAQ</a></li><li class="chapter-item expanded "><div>Scripts</div></li></ol></li><li class="chapter-item expanded "><li class="part-title">Development</li><li class="chapter-item expanded "><a href="../development/contributing_guide.html">Contributing Guide</a></li><li class="chapter-item expanded "><a href="../code_style.html">Code Style</a></li><li class="chapter-item expanded "><a href="../development/reviews.html" class="active">Reviewing Code</a></li><li class="chapter-item expanded "><a href="../development/releases.html">Release Cycle</a></li><li class="chapter-item expanded "><a href="../development/git.html">Git Usage</a></li><li class="chapter-item expanded "><div>Testing</div></li><li><ol class="section"><li class="chapter-item expanded "><a href="../development/demo.html">Demo scripts</a></li></ol></li><li class="chapter-item expanded "><a href="../opentracing.html">OpenTracing</a></li><li class="chapter-item expanded "><a href="../development/database_schema.html">Database Schemas</a></li><li class="chapter-item expanded "><a href="../development/experimental_features.html">Experimental features</a></li><li class="chapter-item expanded "><a href="../development/dependencies.html">Dependency management</a></li><li class="chapter-item expanded "><div>Synapse Architecture</div></li><li><ol class="section"><li class="chapter-item expanded "><a href="../development/synapse_architecture/cancellation.html">Cancellation</a></li><li class="chapter-item expanded "><a href="../log_contexts.html">Log Contexts</a></li><li class="chapter-item expanded "><a href="../replication.html">Replication</a></li><li class="chapter-item expanded "><a href="../tcp_replication.html">TCP Replication</a></li></ol></li><li class="chapter-item expanded "><a href="../development/internal_documentation/index.html">Internal Documentation</a></li><li><ol class="section"><li class="chapter-item expanded "><div>Single Sign-On</div></li><li><ol class="section"><li class="chapter-item expanded "><a href="../development/saml.html">SAML</a></li><li class="chapter-item expanded "><a href="../development/cas.html">CAS</a></li></ol></li><li class="chapter-item expanded "><a href="../development/room-dag-concepts.html">Room DAG concepts</a></li><li class="chapter-item expanded "><div>State Resolution</div></li><li><ol class="section"><li class="chapter-item expanded "><a href="../auth_chain_difference_algorithm.html">The Auth Chain Difference Algorithm</a></li></ol></li><li class="chapter-item expanded "><a href="../media_repository.html">Media Repository</a></li><li class="chapter-item expanded "><a href="../room_and_user_statistics.html">Room and User Statistics</a></li></ol></li><li class="chapter-item expanded "><div>Scripts</div></li><li class="chapter-item expanded affix "><li class="part-title">Other</li><li class="chapter-item expanded "><a href="../deprecation_policy.html">Dependency Deprecation Policy</a></li><li class="chapter-item expanded "><a href="../other/running_synapse_on_single_board_computers.html">Running Synapse on a Single-Board Computer</a></li></ol> + </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> \ No newline at end of file |