Skip to content

docs: Cleanup more literals in API docs #22174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
htuch merged 1 commit into from
Jul 15, 2022
Merged

docs: Cleanup more literals in API docs #22174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions api/envoy/admin/v3/clusters.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ message ClusterStatus {
// The success rate threshold used in the last interval.
// If
// :ref:`outlier_detection.split_external_local_origin_errors<envoy_v3_api_field_config.cluster.v3.OutlierDetection.split_external_local_origin_errors>`
// is *false*, all errors: externally and locally generated were used to calculate the threshold.
// is ``false``, all errors: externally and locally generated were used to calculate the threshold.
// If
// :ref:`outlier_detection.split_external_local_origin_errors<envoy_v3_api_field_config.cluster.v3.OutlierDetection.split_external_local_origin_errors>`
// is *true*, only externally generated errors were used to calculate the threshold.
// is ``true``, only externally generated errors were used to calculate the threshold.
// The threshold is used to eject hosts based on their success rate. See
// :ref:`Cluster outlier detection <arch_overview_outlier_detection>` documentation for details.
//
Expand All @@ -66,7 +66,7 @@ message ClusterStatus {
// taken into account and externally originated errors were treated as success.
// This field should be interpreted only when
// :ref:`outlier_detection.split_external_local_origin_errors<envoy_v3_api_field_config.cluster.v3.OutlierDetection.split_external_local_origin_errors>`
// is *true*. The threshold is used to eject hosts based on their success rate.
// is ``true``. The threshold is used to eject hosts based on their success rate.
// See :ref:`Cluster outlier detection <arch_overview_outlier_detection>` documentation for
// details.
//
Expand Down Expand Up @@ -103,10 +103,10 @@ message HostStatus {
// Request success rate for this host over the last calculated interval.
// If
// :ref:`outlier_detection.split_external_local_origin_errors<envoy_v3_api_field_config.cluster.v3.OutlierDetection.split_external_local_origin_errors>`
// is *false*, all errors: externally and locally generated were used in success rate
// is ``false``, all errors: externally and locally generated were used in success rate
// calculation. If
// :ref:`outlier_detection.split_external_local_origin_errors<envoy_v3_api_field_config.cluster.v3.OutlierDetection.split_external_local_origin_errors>`
// is *true*, only externally generated errors were used in success rate calculation.
// is ``true``, only externally generated errors were used in success rate calculation.
// See :ref:`Cluster outlier detection <arch_overview_outlier_detection>` documentation for
// details.
//
Expand All @@ -129,7 +129,7 @@ message HostStatus {
// errors were treated as success.
// This field should be interpreted only when
// :ref:`outlier_detection.split_external_local_origin_errors<envoy_v3_api_field_config.cluster.v3.OutlierDetection.split_external_local_origin_errors>`
// is *true*.
// is ``true``.
// See :ref:`Cluster outlier detection <arch_overview_outlier_detection>` documentation for
// details.
//
Expand Down
14 changes: 7 additions & 7 deletions api/envoy/admin/v3/config_dump.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ message ConfigDump {
// The following configurations are currently supported and will be dumped in the order given
// below:
//
// * *bootstrap*: :ref:`BootstrapConfigDump <envoy_v3_api_msg_admin.v3.BootstrapConfigDump>`
// * *clusters*: :ref:`ClustersConfigDump <envoy_v3_api_msg_admin.v3.ClustersConfigDump>`
// * *endpoints*: :ref:`EndpointsConfigDump <envoy_v3_api_msg_admin.v3.EndpointsConfigDump>`
// * *listeners*: :ref:`ListenersConfigDump <envoy_v3_api_msg_admin.v3.ListenersConfigDump>`
// * *scoped_routes*: :ref:`ScopedRoutesConfigDump <envoy_v3_api_msg_admin.v3.ScopedRoutesConfigDump>`
// * *routes*: :ref:`RoutesConfigDump <envoy_v3_api_msg_admin.v3.RoutesConfigDump>`
// * *secrets*: :ref:`SecretsConfigDump <envoy_v3_api_msg_admin.v3.SecretsConfigDump>`
// * ``bootstrap``: :ref:`BootstrapConfigDump <envoy_v3_api_msg_admin.v3.BootstrapConfigDump>`
// * ``clusters``: :ref:`ClustersConfigDump <envoy_v3_api_msg_admin.v3.ClustersConfigDump>`
// * ``endpoints``: :ref:`EndpointsConfigDump <envoy_v3_api_msg_admin.v3.EndpointsConfigDump>`
// * ``listeners``: :ref:`ListenersConfigDump <envoy_v3_api_msg_admin.v3.ListenersConfigDump>`
// * ``scoped_routes``: :ref:`ScopedRoutesConfigDump <envoy_v3_api_msg_admin.v3.ScopedRoutesConfigDump>`
// * ``routes``: :ref:`RoutesConfigDump <envoy_v3_api_msg_admin.v3.RoutesConfigDump>`
// * ``secrets``: :ref:`SecretsConfigDump <envoy_v3_api_msg_admin.v3.SecretsConfigDump>`
//
// EDS Configuration will only be dumped by using parameter ``?include_eds``
//
Expand Down
10 changes: 5 additions & 5 deletions api/envoy/admin/v3/config_dump_shared.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ message ListenersConfigDump {
DynamicListenerState draining_state = 4;

// Set if the last update failed, cleared after the next successful update.
// The *error_state* field contains the rejected version of this particular
// The ``error_state`` field contains the rejected version of this particular
// resource along with the reason and timestamp. For successfully updated or
// acknowledged resource, this field should be empty.
UpdateFailureState error_state = 5;
Expand Down Expand Up @@ -182,7 +182,7 @@ message ClustersConfigDump {
google.protobuf.Timestamp last_updated = 3;

// Set if the last update failed, cleared after the next successful update.
// The *error_state* field contains the rejected version of this particular
// The ``error_state`` field contains the rejected version of this particular
// resource along with the reason and timestamp. For successfully updated or
// acknowledged resource, this field should be empty.
// [#not-implemented-hide:]
Expand Down Expand Up @@ -249,7 +249,7 @@ message RoutesConfigDump {
google.protobuf.Timestamp last_updated = 3;

// Set if the last update failed, cleared after the next successful update.
// The *error_state* field contains the rejected version of this particular
// The ``error_state`` field contains the rejected version of this particular
// resource along with the reason and timestamp. For successfully updated or
// acknowledged resource, this field should be empty.
// [#not-implemented-hide:]
Expand Down Expand Up @@ -309,7 +309,7 @@ message ScopedRoutesConfigDump {
google.protobuf.Timestamp last_updated = 4;

// Set if the last update failed, cleared after the next successful update.
// The *error_state* field contains the rejected version of this particular
// The ``error_state`` field contains the rejected version of this particular
// resource along with the reason and timestamp. For successfully updated or
// acknowledged resource, this field should be empty.
// [#not-implemented-hide:]
Expand Down Expand Up @@ -353,7 +353,7 @@ message EndpointsConfigDump {
google.protobuf.Timestamp last_updated = 3;

// Set if the last update failed, cleared after the next successful update.
// The *error_state* field contains the rejected version of this particular
// The ``error_state`` field contains the rejected version of this particular
// resource along with the reason and timestamp. For successfully updated or
// acknowledged resource, this field should be empty.
// [#not-implemented-hide:]
Expand Down
6 changes: 3 additions & 3 deletions api/envoy/config/accesslog/v3/accesslog.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ message RuntimeFilter {
"envoy.config.filter.accesslog.v2.RuntimeFilter";

// Runtime key to get an optional overridden numerator for use in the
// *percent_sampled* field. If found in runtime, this value will replace the
// ``percent_sampled`` field. If found in runtime, this value will replace the
// default numerator.
string runtime_key = 1 [(validate.rules).string = {min_len: 1}];

Expand All @@ -165,9 +165,9 @@ message RuntimeFilter {
// is present, the filter will consistently sample across multiple hosts based
// on the runtime key value and the value extracted from
// :ref:`x-request-id<config_http_conn_man_headers_x-request-id>`. If it is
// missing, or *use_independent_randomness* is set to true, the filter will
// missing, or ``use_independent_randomness`` is set to true, the filter will
// randomly sample based on the runtime key value alone.
// *use_independent_randomness* can be used for logging kill switches within
// ``use_independent_randomness`` can be used for logging kill switches within
// complex nested :ref:`AndFilter
// <envoy_v3_api_msg_config.accesslog.v3.AndFilter>` and :ref:`OrFilter
// <envoy_v3_api_msg_config.accesslog.v3.OrFilter>` blocks that are easier to
Expand Down
52 changes: 26 additions & 26 deletions api/envoy/config/bootstrap/v3/bootstrap.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ message Bootstrap {

// Optional watchdog configuration.
// This is for a single watchdog configuration for the entire system.
// Deprecated in favor of *watchdogs* which has finer granularity.
// Deprecated in favor of ``watchdogs`` which has finer granularity.
Watchdog watchdog = 8
[deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"];

Expand Down Expand Up @@ -249,7 +249,7 @@ message Bootstrap {
// when :ref:`dns_resolvers <envoy_v3_api_field_config.cluster.v3.Cluster.dns_resolvers>` and
// :ref:`use_tcp_for_dns_lookups <envoy_v3_api_field_config.cluster.v3.Cluster.use_tcp_for_dns_lookups>` are
// specified.
// This field is deprecated in favor of *dns_resolution_config*
// This field is deprecated in favor of ``dns_resolution_config``
// which aggregates all of the DNS resolver configuration in a single message.
bool use_tcp_for_dns_lookups = 20
[deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"];
Expand All @@ -267,12 +267,12 @@ message Bootstrap {
// or any other DNS resolver types and the related parameters.
// For example, an object of
// :ref:`CaresDnsResolverConfig <envoy_v3_api_msg_extensions.network.dns_resolver.cares.v3.CaresDnsResolverConfig>`
// can be packed into this *typed_dns_resolver_config*. This configuration replaces the
// can be packed into this ``typed_dns_resolver_config``. This configuration replaces the
// :ref:`dns_resolution_config <envoy_v3_api_field_config.bootstrap.v3.Bootstrap.dns_resolution_config>`
// configuration.
// During the transition period when both *dns_resolution_config* and *typed_dns_resolver_config* exists,
// when *typed_dns_resolver_config* is in place, Envoy will use it and ignore *dns_resolution_config*.
// When *typed_dns_resolver_config* is missing, the default behavior is in place.
// During the transition period when both ``dns_resolution_config`` and ``typed_dns_resolver_config`` exists,
// when ``typed_dns_resolver_config`` is in place, Envoy will use it and ignore ``dns_resolution_config``.
// When ``typed_dns_resolver_config`` is missing, the default behavior is in place.
Copy link
Copy Markdown
Member

@htuch htuch Jul 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this covered by the RST warnings? Is there a style guide entry here?

Copy link
Copy Markdown
Member Author

@phlax phlax Jul 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how could it - this is me editing prose to make words that should be literal, so - until we get AI in the pipeline this is not something we can test for

more generally - the use of * for literals relates to historic advice which i changed long ago

we could perhaps check for *word* and spellcheck the word as literals should not be marked up this way - it would catch some - ill look at it when i get to the spellcheck stuff

Copy link
Copy Markdown
Contributor

@adisuissa adisuissa Jul 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess an alternative would be to clarify it in https://github.com/envoyproxy/envoy/blob/main/api/STYLE.md, but as long as we don't detect this in tooling, it will be hard to catch.
Can we hook a validation to the comments parser somehow?

Copy link
Copy Markdown
Member Author

@phlax phlax Jul 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we hook a validation to the comments parser somehow?

this is what we do now for single backticks

for * or bare literals its much harder to detect - there are genuine uses of * - ie for italics/emphasis - and there is nothing to grep for with bare literals

spellchecking could work and catch most issues but not all and with some false +ves etc

this is something i intend to work, but not imminently - some preparatory work has been done over in pytooling tho

wrt to the STYLE guide i can update/review if you think it needs it but would suggest we follow up with that

Copy link
Copy Markdown
Member Author

@phlax phlax Jul 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also - mho is that the best style guide is what is there - the more we clean stuff up the more people are working from examples that exemplify what we want

Copy link
Copy Markdown
Contributor

@adisuissa adisuissa Jul 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also - mho is that the best style guide is what is there - the more we clean stuff up the more people are working from examples that exemplify what we want

I also tend to follow other examples.

// [#extension-category: envoy.network.dns_resolver]
core.v3.TypedExtensionConfig typed_dns_resolver_config = 31;

Expand All @@ -289,18 +289,18 @@ message Bootstrap {
// xdstp:// URL authority resolution. The algorithm is as
// follows:
// 1. The authority field is taken from the xdstp:// URL, call
// this *resource_authority*.
// 2. *resource_authority* is compared against the authorities in any peer
// *ConfigSource*. The peer *ConfigSource* is the configuration source
// this ``resource_authority``.
// 2. ``resource_authority`` is compared against the authorities in any peer
// ``ConfigSource``. The peer ``ConfigSource`` is the configuration source
// message which would have been used unconditionally for resolution
// with opaque resource names. If there is a match with an authority, the
// peer *ConfigSource* message is used.
// 3. *resource_authority* is compared sequentially with the authorities in
// each configuration source in *config_sources*. The first *ConfigSource*
// peer ``ConfigSource`` message is used.
// 3. ``resource_authority`` is compared sequentially with the authorities in
// each configuration source in ``config_sources``. The first ``ConfigSource``
// to match wins.
// 4. As a fallback, if no configuration source matches, then
// *default_config_source* is used.
// 5. If *default_config_source* is not specified, resolution fails.
// ``default_config_source`` is used.
// 5. If ``default_config_source`` is not specified, resolution fails.
// [#not-implemented-hide:]
repeated core.v3.ConfigSource config_sources = 22;

Expand Down Expand Up @@ -350,7 +350,7 @@ message Admin {
// The path to write the access log for the administration server. If no
// access log is desired specify ‘/dev/null’. This is only required if
// :ref:`address <envoy_v3_api_field_config.bootstrap.v3.Admin.address>` is set.
// Deprecated in favor of *access_log* which offers more options.
// Deprecated in favor of ``access_log`` which offers more options.
string access_log_path = 1
[deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"];

Expand Down Expand Up @@ -392,7 +392,7 @@ message ClusterManager {
// Name of the local cluster (i.e., the cluster that owns the Envoy running
// this configuration). In order to enable :ref:`zone aware routing
// <arch_overview_load_balancing_zone_aware_routing>` this option must be set.
// If *local_cluster_name* is defined then :ref:`clusters
// If ``local_cluster_name`` is defined then :ref:`clusters
// <envoy_v3_api_msg_config.cluster.v3.Cluster>` must be defined in the :ref:`Bootstrap
// static cluster resources
// <envoy_v3_api_field_config.bootstrap.v3.Bootstrap.StaticResources.clusters>`. This is unrelated to
Expand All @@ -408,7 +408,7 @@ message ClusterManager {
core.v3.BindConfig upstream_bind_config = 3;

// A management server endpoint to stream load stats to via
// *StreamLoadStats*. This must have :ref:`api_type
// ``StreamLoadStats``. This must have :ref:`api_type
// <envoy_v3_api_field_config.core.v3.ApiConfigSource.api_type>` :ref:`GRPC
// <envoy_v3_api_enum_value_config.core.v3.ApiConfigSource.ApiType.GRPC>`.
core.v3.ApiConfigSource load_stats_config = 4;
Expand Down Expand Up @@ -454,15 +454,15 @@ message Watchdog {
}

// Register actions that will fire on given WatchDog events.
// See *WatchDogAction* for priority of events.
// See ``WatchDogAction`` for priority of events.
repeated WatchdogAction actions = 7;

// The duration after which Envoy counts a nonresponsive thread in the
// *watchdog_miss* statistic. If not specified the default is 200ms.
// ``watchdog_miss`` statistic. If not specified the default is 200ms.
google.protobuf.Duration miss_timeout = 1;

// The duration after which Envoy counts a nonresponsive thread in the
// *watchdog_mega_miss* statistic. If not specified the default is
// ``watchdog_mega_miss`` statistic. If not specified the default is
// 1000ms.
google.protobuf.Duration megamiss_timeout = 2;

Expand All @@ -471,20 +471,20 @@ message Watchdog {
// kill behavior. If not specified the default is 0 (disabled).
google.protobuf.Duration kill_timeout = 3;

// Defines the maximum jitter used to adjust the *kill_timeout* if *kill_timeout* is
// Defines the maximum jitter used to adjust the ``kill_timeout`` if ``kill_timeout`` is
// enabled. Enabling this feature would help to reduce risk of synchronized
// watchdog kill events across proxies due to external triggers. Set to 0 to
// disable. If not specified the default is 0 (disabled).
google.protobuf.Duration max_kill_timeout_jitter = 6 [(validate.rules).duration = {gte {}}];

// If max(2, ceil(registered_threads * Fraction(*multikill_threshold*)))
// If ``max(2, ceil(registered_threads * Fraction(*multikill_threshold*)))``
// threads have been nonresponsive for at least this duration kill the entire
// Envoy process. Set to 0 to disable this behavior. If not specified the
// default is 0 (disabled).
google.protobuf.Duration multikill_timeout = 4;

// Sets the threshold for *multikill_timeout* in terms of the percentage of
// nonresponsive threads required for the *multikill_timeout*.
// Sets the threshold for ``multikill_timeout`` in terms of the percentage of
// nonresponsive threads required for the ``multikill_timeout``.
// If not specified the default is 0.
type.v3.Percent multikill_threshold = 5;
}
Expand All @@ -495,7 +495,7 @@ message Watchdog {
// have an out of band system to terminate the process.
//
// The interface for the extension is ``Envoy::Server::Configuration::FatalAction``.
// *FatalAction* extensions live in the ``envoy.extensions.fatal_actions`` API
// ``FatalAction`` extensions live in the ``envoy.extensions.fatal_actions`` API
// namespace.
message FatalAction {
// Extension specific configuration for the action. It's expected to conform
Expand Down Expand Up @@ -575,7 +575,7 @@ message RuntimeLayer {
option (udpa.annotations.versioning).previous_message_type =
"envoy.config.bootstrap.v2.RuntimeLayer.RtdsLayer";

// Resource to subscribe to at *rtds_config* for the RTDS layer.
// Resource to subscribe to at ``rtds_config`` for the RTDS layer.
string name = 1;

// RTDS configuration source.
Expand Down
Loading