chore: remove REVIEW comments

Remove all // REVIEW: comments that were added to aid code review.
These served their purpose during the review process and are no
longer needed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: adwk67

rust/operator-binary/src/controller.rs

@@ -159,9 +159,6 @@ impl ValidatedLogging {
     }
 }
 
-// REVIEW: ValidatedAirflowCluster is the central validated type. It holds all data needed
-// by the build stage so that build() can be infallible. All optional-after-merge fields
-// are unwrapped during validation, and logging is pre-validated into ValidatedLogging.
 #[derive(Clone)]
 pub struct ValidatedAirflowCluster {
     metadata: ObjectMeta,
@@ -303,12 +300,6 @@ impl ReconcilerError for Error {
     }
 }
 
-// REVIEW: The reconcile pipeline is structured as five sequential stages:
-// 1. dereference — async, fallible: resolve external references
-// 2. validate — sync, fallible: validate and merge configs
-// 3. build — sync, infallible: construct Kubernetes resources
-// 4. apply — async, fallible: apply resources to the cluster
-// 5. update_status — async, fallible: patch status on the CRD
 pub async fn reconcile(
     airflow: Arc<DeserializeGuard<v1alpha2::AirflowCluster>>,
     ctx: Arc<Ctx>,
@@ -336,9 +327,6 @@ pub async fn reconcile(
     let validated =
         validate_cluster(airflow, &dereferenced, &ctx.product_config).context(ValidateSnafu)?;
 
-    // REVIEW: build() is infallible — all validation and fallible operations (config
-    // generation, PodBuilder/ContainerBuilder usage, logging validation) happen in the
-    // validate stage. The build stage purely assembles Kubernetes resource structs.
     // --- build (sync, infallible) ---
     let prepared = build(&validated);
 

rust/operator-binary/src/controller/build.rs

@@ -33,9 +33,6 @@ fn main_container_for_role(_role: &AirflowRole) -> Container {
     Container::Airflow
 }
 
-// REVIEW: build() is infallible. All validation and fallible operations (config generation,
-// PodBuilder/ContainerBuilder usage, logging validation) are performed in the validate
-// stage. The build stage purely assembles Kubernetes resource structs from validated data.
 pub(crate) fn build(validated: &ValidatedAirflowCluster) -> KubernetesResources<Prepared> {
     let mut stateful_sets = Vec::new();
     let mut config_maps = Vec::new();
@@ -140,9 +137,6 @@ fn build_pdb(
         _ => 1,
     });
 
-    // REVIEW: from_str_unsafe is used here because the values come from constants (APP_NAME,
-    // OPERATOR_NAME, AIRFLOW_CONTROLLER_NAME) or validated role names — they are known to be
-    // valid at compile time or have been validated during the validate stage.
     Some({
         framework::builder::pdb::pod_disruption_budget_builder_with_role(
             cluster,
@@ -240,11 +234,6 @@ fn build_metrics_service(
     }
 }
 
-// REVIEW: from_str_unsafe is used for label construction throughout these helpers because
-// the inputs are either compile-time constants (APP_NAME, OPERATOR_NAME, etc.) or values
-// that have already been validated during the validate stage (role names, role group names,
-// product versions). Using from_str_unsafe avoids redundant re-validation in the infallible
-// build stage.
 pub(super) fn build_recommended_labels(
     cluster: &ValidatedAirflowCluster,
     role: &str,

rust/operator-binary/src/controller/build/role_group_builder.rs

@@ -359,10 +359,6 @@ impl<'a> RoleGroupBuilder<'a> {
     }
 
     fn build_volumes(&self) -> Vec<Volume> {
-        // REVIEW: controller_commons::create_volumes is called with the new validated type
-        // (&ValidatedContainerLogConfigChoice) instead of the old Option<&ContainerLogConfig>.
-        // This is safe because the logging config has already been validated during the
-        // validate stage.
         let mut volumes = controller_commons::create_volumes(
             &self.rolegroup_ref.object_name(),
             &self.role_group_config.logging.airflow_container,

rust/operator-binary/src/controller/validate.rs

@@ -793,9 +793,6 @@ fn build_executor_template_config_maps(
         pb.add_container(airflow_container.build());
         pb.add_volumes(airflow.volumes().clone())
             .context(AddVolumeSnafu)?;
-        // REVIEW: controller_commons::create_volumes now takes &ValidatedContainerLogConfigChoice
-        // instead of Option<&ContainerLogConfig>. The validated type ensures logging config
-        // has already been checked, so the build stage can use it directly.
         pb.add_volumes(controller_commons::create_volumes(
             &executor_rolegroup_ref.object_name(),
             &validated_config.logging.airflow_container,

rust/operator-binary/src/controller_commons.rs

@@ -13,8 +13,6 @@ pub const CONFIG_VOLUME_NAME: &str = "config";
 pub const LOG_CONFIG_VOLUME_NAME: &str = "log-config";
 pub const LOG_VOLUME_NAME: &str = "log";
 
-// REVIEW: parameter changed from Option<&ContainerLogConfig> to &ValidatedContainerLogConfigChoice.
-// Pattern matching is simpler now because we match on a flat enum instead of nested Option<ContainerLogConfig>.
 pub fn create_volumes(
     config_map_name: &str,
     log_config: &ValidatedContainerLogConfigChoice,

rust/operator-binary/src/crd/mod.rs

@@ -571,8 +571,6 @@ pub struct AirflowOpaConfig {
     pub cache: UserInformationCache,
 }
 
-// REVIEW: Ord/PartialOrd added so AirflowRole can be used as a BTreeMap key
-// in the new controller's ValidatedAirflowCluster (BTreeMap<AirflowRole, ValidatedRoleConfig>)
 #[derive(
     Clone,
     Debug,

rust/operator-binary/src/env_vars.rs

@@ -373,7 +373,6 @@ fn static_envs(
 
 /// Return environment variables to be applied to the configuration map used in conjunction with
 /// the `kubernetesExecutor` worker.
-// REVIEW: simplified from &ExecutorConfig — only config.logging.enable_vector_agent was used
 pub fn build_airflow_template_envs(
     airflow: &v1alpha2::AirflowCluster,
     env_overrides: &HashMap<String, String>,

rust/operator-binary/src/framework/types/operator.rs

@@ -12,7 +12,6 @@ use crate::attributed_string_type;
 attributed_string_type! {
     ProductName,
     "The name of a product",
-    // REVIEW: example is operator-specific — parameterise when moving to operator-rs
     "airflow",
     // A suffix is added to produce a label value. An according compile-time check ensures that
     // max_length cannot be set higher.
@@ -31,13 +30,10 @@ attributed_string_type! {
 attributed_string_type! {
     ClusterName,
     "The name of a cluster/stacklet",
-    // REVIEW: example is operator-specific — parameterise when moving to operator-rs
     "my-airflow-cluster",
     // Suffixes are added to produce resource names. According compile-time checks ensure that
     // max_length cannot be set higher. Reduced from opensearch's 24 to 22 because airflow's
     // longest role name ("dagprocessor") is 12 chars vs opensearch's 10.
-    // REVIEW: max_length is operator-specific (depends on longest RoleName) — parameterise
-    // when moving to operator-rs
     (max_length = 22),
     is_rfc_1035_label_name,
     is_valid_label_value
@@ -46,15 +42,13 @@ attributed_string_type! {
 attributed_string_type! {
     ControllerName,
     "The name of a controller in an operator",
-    // REVIEW: example is operator-specific — parameterise when moving to operator-rs
     "airflowcluster",
     is_valid_label_value
 }
 
 attributed_string_type! {
     OperatorName,
     "The name of an operator",
-    // REVIEW: example is operator-specific — parameterise when moving to operator-rs
     "airflow.stackable.tech",
     is_valid_label_value
 }
@@ -66,8 +60,6 @@ attributed_string_type! {
     // The role-group name is used to produce resource names. To make sure that all resource names
     // are valid, max_length is restricted. Compile-time checks ensure that max_length cannot be
     // set higher if not other names like the RoleName are set lower accordingly.
-    // REVIEW: max_length is operator-specific (depends on ClusterName and RoleName budgets) —
-    // parameterise when moving to operator-rs
     (max_length = 16),
     is_rfc_1123_label_name,
     is_valid_label_value
@@ -76,13 +68,10 @@ attributed_string_type! {
 attributed_string_type! {
     RoleName,
     "The name of a role name",
-    // REVIEW: example is operator-specific — parameterise when moving to operator-rs
     "webserver",
     // The role name is used to produce resource names. To make sure that all resource names are
     // valid, max_length is restricted. Compile-time checks ensure that max_length cannot be set
     // higher if not other names like the RoleGroupName are set lower accordingly.
-    // REVIEW: max_length is operator-specific (airflow needs 12 for "dagprocessor", opensearch
-    // uses 10) — parameterise when moving to operator-rs
     (max_length = 12),
     is_rfc_1123_label_name,
     is_valid_label_value

rust/operator-binary/src/main.rs

@@ -177,7 +177,6 @@ async fn main() -> anyhow::Result<()> {
                 )
                 .graceful_shutdown_on(sigterm_watcher.handle())
                 .run(
-                    // REVIEW: renamed from reconcile_airflow to reconcile
                     controller::reconcile,
                     controller::error_policy,
                     Arc::new(controller::Ctx {

rust/operator-binary/src/product_logging.rs

@@ -18,8 +18,6 @@ use crate::{
 const LOG_CONFIG_FILE: &str = "log_config.py";
 const LOG_FILE: &str = "airflow.py.json";
 
-// REVIEW: signature changed from Logging<C> + two container keys to pre-validated types.
-// The function is now infallible — validation happens earlier in the pipeline.
 /// Extend the ConfigMap with logging and Vector configurations
 pub fn extend_config_map_with_log_config<K>(
     rolegroup: &RoleGroupRef<K>,