Skip to content

Commit f689dd3

Browse files
Apply suggestions from code review
Co-authored-by: Nick <[email protected]>
1 parent af6f830 commit f689dd3

File tree

2 files changed

+3
-9
lines changed

2 files changed

+3
-9
lines changed

crates/stackable-operator/src/builder/pod/container.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ pub enum Error {
3838
/// This will automatically create the necessary volumes and mounts for each `ConfigMap` which is added.
3939
///
4040
/// This struct is often times using an [`IndexMap`] to have consistent ordering (so we don't produce reconcile loops).
41-
/// We are also choosing it over an [`std::collections::BTreeMap`], as it's easier to debug for users, as logically
41+
/// We are also choosing it over a [`BTreeMap`], because it is easier to debug for users, as logically
4242
/// grouped volumeMounts (e.g. all volumeMounts related to S3) are near each other in the list instead of "just" being
4343
/// sorted alphabetically.
4444
#[derive(Clone, Default)]
@@ -212,9 +212,6 @@ impl ContainerBuilder {
212212
/// Historically this function unconditionally added volumeMounts, which resulted in invalid
213213
/// [`k8s_openapi::api::core::v1::PodSpec`]s, as volumeMounts where added multiple times - think of Trino using the same [`crate::commons::s3::S3Connection`]
214214
/// two times, resulting in e.g. the s3 credentials being mounted twice as the same volumeMount.
215-
///
216-
/// We could have made this function fallible, but decided not to do so for now, as it would be a bigger breaking
217-
/// change.
218215
pub fn add_volume_mount_struct(&mut self, volume_mount: VolumeMount) -> Result<&mut Self> {
219216
if let Some(existing_volume_mount) = self.volume_mounts.get(&volume_mount.mount_path) {
220217
ensure!(

crates/stackable-operator/src/builder/pod/mod.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ pub enum Error {
5252
#[snafu(display("object is missing key {key:?}"))]
5353
MissingObjectKey { key: &'static str },
5454

55-
#[snafu(display("The volume {volume_name:?} is clashing with an already existing volume with the same name but a different content. \
55+
#[snafu(display("The volume {volume_name:?} clashes with an existing volume of the same name but with different content. \
5656
The existing volume is {existing_volume:?}, the new one is {new_volume:?}"))]
5757
ClashingVolumeName {
5858
volume_name: String,
@@ -64,7 +64,7 @@ pub enum Error {
6464
/// A builder to build [`Pod`] or [`PodTemplateSpec`] objects.
6565
///
6666
/// This struct is often times using an [`IndexMap`] to have consistent ordering (so we don't produce reconcile loops).
67-
/// We are also choosing it over an [`BTreeMap`], as it's easier to debug for users, as logically grouped volumes
67+
/// We are also choosing it over a [`BTreeMap`], because it is easier to debug for users, as logically grouped volumes
6868
/// (e.g. all volumes related to S3) are near each other in the list instead of "just" being sorted alphabetically.
6969
#[derive(Clone, Debug, Default, PartialEq)]
7070
pub struct PodBuilder {
@@ -290,9 +290,6 @@ impl PodBuilder {
290290
/// Historically this function unconditionally added volumes, which resulted in invalid [`PodSpec`]s, as volumes
291291
/// where added multiple times - think of Trino using the same [`crate::commons::s3::S3Connection`] two times,
292292
/// resulting in e.g. the s3 credentials being mounted twice as the sake volume.
293-
///
294-
/// We could have made this function fallible, but decided not to do so for now, as it would be a bigger breaking
295-
/// change.
296293
pub fn add_volume(&mut self, volume: Volume) -> Result<&mut Self> {
297294
if let Some(existing_volume) = self.volumes.get(&volume.name) {
298295
ensure!(

0 commit comments

Comments
 (0)