Unverified Commit dfe7f841 authored by Kevin Chisholm's avatar Kevin Chisholm Committed by GitHub

[Conductor] Add documentation to individual files (#121277)

* add documentation to files

* Update dev/conductor/core/lib/src/stdio.dart
Co-authored-by: 's avatarChristopher Fujino <fujino@google.com>

* Update dev/conductor/core/lib/src/start.dart
Co-authored-by: 's avatarChristopher Fujino <fujino@google.com>

* Update dev/conductor/core/lib/src/repository.dart
Co-authored-by: 's avatarChristopher Fujino <fujino@google.com>

* Update dev/conductor/core/lib/src/next.dart
Co-authored-by: 's avatarChristopher Fujino <fujino@google.com>

* Update dev/conductor/core/lib/src/git.dart
Co-authored-by: 's avatarChristopher Fujino <fujino@google.com>

* Update dev/conductor/core/lib/src/git.dart
Co-authored-by: 's avatarChristopher Fujino <fujino@google.com>

* Update stdio.dart

---------
Co-authored-by: 's avatarChristopher Fujino <fujino@google.com>
parent 219ff645
......@@ -9,6 +9,12 @@ import 'package:process/process.dart';
import './globals.dart';
/// A wrapper around git process calls that can be mocked for unit testing.
///
/// The `Git` class is a relatively (compared to `Repository`) lightweight
/// abstraction over invocations to the `git` cli tool. The main
/// motivation for creating this class was so that it could be overridden in
/// tests. However, now that tests rely on the [FakeProcessManager] this
/// abstraction is redundant.
class Git {
const Git(this.processManager);
......
......@@ -18,6 +18,11 @@ const String kStateOption = 'state-file';
const String kYesFlag = 'yes';
/// Command to proceed from one [pb.ReleasePhase] to the next.
///
/// After `conductor start`, the rest of the release steps are initiated by the
/// user via `conductor next`. Thus this command's behavior is conditional upon
/// which phase of the release the user is currently in. This is implemented
/// with a switch case statement.
class NextCommand extends Command<void> {
NextCommand({
required this.checkouts,
......
......@@ -60,6 +60,39 @@ class Remote {
}
/// A source code repository.
///
/// This class is an abstraction over a git
/// repository on the local disk. Ideally this abstraction would hide from
/// the outside libraries what git calls were needed to either read or update
/// data in the underlying repository. In practice, most of the bugs in the
/// conductor codebase are related to the git calls made from this and its
/// subclasses.
///
/// Two factors that make this code more complicated than it would otherwise
/// need to be are:
/// 1. That any particular invocation of the conductor may or may not already
/// have the git checkout present on disk, depending on what commands were
/// previously run; and
/// 2. The need to provide overrides for integration tests (in particular
/// the ability to mark a [Repository] instance as a [localUpstream] made
/// integration tests more hermetic, at the cost of complexity in the
/// implementation).
///
/// The only way to simplify the first factor would be to change the behavior of
/// the conductor tool to be a long-lived dart process that keeps all of its
/// state in memory and blocks on user input. This would add the constraint that
/// the user would need to keep the process running for the duration of a
/// release, which could potentially take multiple days and users could not
/// manually change the state of the release process (via editing the JSON
/// config file). However, these may be reasonable trade-offs to make the
/// codebase simpler and easier to reason about.
///
/// The way to simplify the second factor would be to not put any special
/// handling in this library for integration tests. This would make integration
/// tests more difficult/less hermetic, but the production code more reliable.
/// This is probably the right trade-off to make, as the integration tests were
/// still not hermetic or reliable, and the main integration test was ultimately
/// deleted in #84354.
abstract class Repository {
Repository({
required this.name,
......
......@@ -34,6 +34,16 @@ const String kVersionOverrideOption = 'version-override';
const String kGithubUsernameOption = 'github-username';
/// Command to print the status of the current Flutter release.
///
/// This command has many required options which the user must provide
/// via command line arguments (or optionally environment variables).
///
/// This command is the one with the worst user experience (as the user has to
/// carefully type out many different options into their terminal) and the one
/// that would benefit the most from a GUI frontend. This command will
/// optionally read its options from an environment variable to facilitate a workflow
/// in which configuration is provided by editing a bash script that sets environment
/// variables and then invokes the conductor tool.
class StartCommand extends Command<void> {
StartCommand({
required this.checkouts,
......
......@@ -33,6 +33,19 @@ const String stablePostReleaseMsg = """
'\t 4. Post announcement flutter release hotline chat room',
'\t\t Chatroom: ${globals.flutterReleaseHotline}',
""";
// The helper functions in `state.dart` wrap the code-generated dart files in
// `lib/src/proto/`. The most interesting of these functions is:
// * `pb.ConductorState readStateFromFile(File)` - uses the code generated
// `.mergeFromProto3Json()` method to deserialize the JSON content from the
// config file into a Dart instance of the `ConductorState` class.
// * `void writeStateFromFile(File, pb.ConductorState, List<String>)`
// - similarly calls the `.toProto3Json()` method to serialize a
// * `ConductorState` instance to a JSON string which is then written to disk.
// `String phaseInstructions(pb.ConductorState state)` - returns instructions
// for what the user is supposed to do next based on `state.currentPhase`.
// * `String presentState(pb.ConductorState state)` - pretty print the state file.
// This is a little easier to read than the raw JSON.
String luciConsoleLink(String channel, String groupName) {
assert(
......
......@@ -6,6 +6,14 @@ import 'dart:io' as io;
import 'package:meta/meta.dart';
/// An interface for presenting text output to the user.
///
/// Although this could have been simplified by calling `print()`
/// from the tool, this abstraction allows unit tests to verify output
/// and allows a GUI frontend to provide an alternative implementation.
///
/// User input probably should be part of this class–however it is currently
/// part of context.dart.
abstract class Stdio {
final List<String> logs = <String>[];
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment