ronantakizawa/codereview-bench huggingface dataset:
Repository: kubernetes/kubernetes
File: staging/src/k8s.io/apiserver/pkg/endpoints/handlers/watch.go (Go)

Code before:
		ws:             ws,
		useTextFraming: useTextFraming,
	}
}

func (w *websocketFramer) Write(p []byte) (int, error) {
	if w.useTextFraming {
		// bytes.Buffer::String() has a special handling of nil value, but given
		// we're writing serialized watch events, this will never happen here.
		if err := websocket.Message.Send(w.ws, string(p)); err != nil {
			return 0, err
		}
		return len(p), nil
	}
	if err := websocket.Message.Send(w.ws, p); err != nil {
		return 0, err
	}
	return len(p), nil
}

var _ io.Writer = &websocketFramer{}

func shouldRecordWatchListLatency(event watch.Event) bool {
	if event.Type != watch.Bookmark || !utilfeature.DefaultFeatureGate.Enabled(features.WatchList) {
		return false
	}
	// as of today the initial-events-end annotation is added only to a single event
	// by the watch cache and only when certain conditions are met
	//
	// for more please read https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/3157-watch-list
	hasAnnotation, err := storage.HasInitialEventsEndBookmarkAnnotation(event.Object)
	if err != nil {
		utilruntime.HandleError(fmt.Errorf("unable to determine if the obj has the required annotation for measuring watchlist latency, obj %T: %v", event.Object, err))
		return false
	}
	return hasAnnotation
}


Review comment (suggestion):
One could argue that this should be a logger because the function doesn't need a context, but then the caller would have to call `klog.FromContext` when typically that's only necessary in case of an error, so I think a context here is fine.


Code after:

func newWebsocketFramer(ws *websocket.Conn, useTextFraming bool) io.Writer {
	return &websocketFramer{
		ws:             ws,
		useTextFraming: useTextFraming,
	}
}

func (w *websocketFramer) Write(p []byte) (int, error) {
	if w.useTextFraming {
		// bytes.Buffer::String() has a special handling of nil value, but given
		// we're writing serialized watch events, this will never happen here.
		if err := websocket.Message.Send(w.ws, string(p)); err != nil {
			return 0, err
		}
		return len(p), nil
	}
	if err := websocket.Message.Send(w.ws, p); err != nil {
		return 0, err
	}
	return len(p), nil
}

var _ io.Writer = &websocketFramer{}

func shouldRecordWatchListLatency(ctx context.Context, event watch.Event) bool {
	if event.Type != watch.Bookmark || !utilfeature.DefaultFeatureGate.Enabled(features.WatchList) {
		return false
	}
	// as of today the initial-events-end annotation is added only to a single event
	// by the watch cache and only when certain conditions are met
	//
	// for more please read https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/3157-watch-list
	hasAnnotation, err := storage.HasInitialEventsEndBookmarkAnnotation(event.Object)
	if err != nil {
		utilruntime.HandleErrorWithContext(ctx, err, "Unable to determine if the object has the required annotation for measuring watchlist latency", "object", fmt.Sprintf("%T", event.Object))
		return false
	}
	return hasAnnotation
}
Repository: kubernetes/kubernetes
File: pkg/features/kube_features.go (Go)

Code before:
	VolumeAttributesClass: {},

	VolumeLimitScaling: {},

	WinDSR: {},

	WinOverlay: {},

	WindowsCPUAndMemoryAffinity: {},

	WindowsGracefulNodeShutdown: {GracefulNodeShutdown},

	WindowsHostNetwork: {},

	apiextensionsfeatures.CRDObservedGenerationTracking: {},

	apiextensionsfeatures.CRDValidationRatcheting: {},

	apiextensionsfeatures.CustomResourceFieldSelectors: {},

	genericfeatures.APIResponseCompression: {},

	genericfeatures.APIServerIdentity: {},

	genericfeatures.APIServerTracing: {},

	genericfeatures.APIServingWithRoutine: {},

	genericfeatures.AggregatedDiscoveryRemoveBetaType: {},

	genericfeatures.AllowParsingUserUIDFromCertAuth: {},

	genericfeatures.AllowUnsafeMalformedObjectDeletion: {},

	genericfeatures.AnonymousAuthConfigurableEndpoints: {},

	genericfeatures.AuthorizeWithSelectors: {},

	genericfeatures.BtreeWatchCache: {},

	genericfeatures.CBORServingAndStorage: {},

	genericfeatures.ConcurrentWatchObjectDecode: {},

	genericfeatures.ConsistentListFromCache: {},

	genericfeatures.ConstrainedImpersonation: {},

	genericfeatures.DeclarativeValidation: {},

	genericfeatures.DeclarativeValidationBeta:     {genericfeatures.DeclarativeValidation},

Review comment (suggestion):
```suggestion
	WorkloadAwarePreemption: {GenericWorkload},
```

Code after:

	TopologyManagerPolicyBetaOptions: {},

	TopologyManagerPolicyOptions: {},

	TranslateStreamCloseWebsocketRequests: {},

	UserNamespacesHostNetworkSupport: {UserNamespacesSupport},

	UserNamespacesSupport: {},

	VolumeAttributesClass: {},

	VolumeLimitScaling: {},

	WinDSR: {},

	WinOverlay: {},

	WindowsCPUAndMemoryAffinity: {},

	WindowsGracefulNodeShutdown: {GracefulNodeShutdown},

	WindowsHostNetwork: {},

	WorkloadAwarePreemption: {GangScheduling},

	apiextensionsfeatures.CRDObservedGenerationTracking: {},

	apiextensionsfeatures.CRDValidationRatcheting: {},

	apiextensionsfeatures.CustomResourceFieldSelectors: {},

	genericfeatures.APIResponseCompression: {},

	genericfeatures.APIServerIdentity: {},

	genericfeatures.APIServerTracing: {},

	genericfeatures.APIServingWithRoutine: {},

	genericfeatures.AggregatedDiscoveryRemoveBetaType: {},

	genericfeatures.AllowParsingUserUIDFromCertAuth: {},

	genericfeatures.AllowUnsafeMalformedObjectDeletion: {},

	genericfeatures.AnonymousAuthConfigurableEndpoints: {},

	genericfeatures.AuthorizeWithSelectors: {},
Repository: kubernetes/kubernetes
File: staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/machine.go (Go)

Code before:
/*
Copyright 2026 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package deviceattribute

import (
	"io/fs"
	"os"
)

type Machine struct {
	FS fs.ReadLinkFS
}

type MachineModifier func(*Machine)

func WithFS(sysfs fs.ReadLinkFS) func(*Machine) {
	return func(mc *Machine) {
		mc.FS = sysfs
	}
}

func WithFSFromRoot(root string) func(*Machine) {
	return func(mc *Machine) {
		mc.FS = os.DirFS(root).(fs.ReadLinkFS)
	}
}


Review comment (suggestion):
Comments would be userful for users on exported type/functions.

Code after:
/*
Copyright The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package deviceattribute

import (
	"io/fs"
	"os"
)

// machine is the bare minimum machine/host abstraction we need.
// we use a wrapping struct to enable the functional option pattern
type machine struct {
	sysfs fs.ReadLinkFS
}

// MachineModifier is the type of functional options
type MachineModifier func(*machine)

// WithFS replaces the host sysfs with a custom FS
func WithFS(sysfs fs.ReadLinkFS) MachineModifier {
	return func(mc *machine) {
		mc.sysfs = sysfs
	}
}

// WithFSFromRoot replaces the host sysfs with a FS whose root
// is the provided path. Like chroot, this is for convenience
// and testability and for usage within containers, not for security.
func WithFSFromRoot(root string) MachineModifier {
	return func(mc *machine) {
		mc.sysfs = os.DirFS(root).(fs.ReadLinkFS)
	}
}
Repository: kubernetes/kubernetes
File: staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/machine.go (Go)

Code before:
/*
Copyright 2026 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package deviceattribute

import (
	"io/fs"
	"os"
)

type Machine struct {
	FS fs.ReadLinkFS
}


Review comment (security):
```suggestion
Copyright The Kubernetes Authors.
```

The year is no longer needed.

Code after:
/*
Copyright The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package deviceattribute

import (
	"io/fs"
	"os"
)

// machine is the bare minimum machine/host abstraction we need.
// we use a wrapping struct to enable the functional option pattern
type machine struct {
	sysfs fs.ReadLinkFS
Repository: launchbadge/sqlx
File: sqlx-core/src/sqlite/connection/collation.rs (Rust)

Code before:

unsafe extern "C" fn free_boxed_value<T>(p: *mut c_void) {
    drop(Box::from_raw(p as *mut T));
}

pub(crate) fn create_collation<F>(
    handle: &ConnectionHandle,
    name: &str,
    collation: F,
) -> Result<(), Error>
where
    F: Fn(&str, &str) -> Ordering + Send + Sync + UnwindSafe + 'static,
{
    unsafe extern "C" fn call_boxed_closure<C>(
        arg1: *mut c_void,
        arg2: c_int,
        arg3: *const c_void,
        arg4: c_int,
        arg5: *const c_void,
    ) -> c_int
    where
        C: Fn(&str, &str) -> Ordering,
    {
        let r = catch_unwind(|| {
            let boxed_f: *mut C = arg1 as *mut C;
            assert!(!boxed_f.is_null(), "Internal error - null function pointer");
            let s1 = {
                let c_slice = slice::from_raw_parts(arg3 as *const u8, arg2 as usize);
                String::from_utf8_lossy(c_slice)
            };
            let s2 = {
                let c_slice = slice::from_raw_parts(arg5 as *const u8, arg4 as usize);
                String::from_utf8_lossy(c_slice)
            };
            (*boxed_f)(s1.as_ref(), s2.as_ref())
        });
        let t = match r {
            Err(_) => {
                return -1; // FIXME How ?
            }
            Ok(r) => r,
        };

        match t {
            Ordering::Less => -1,
            Ordering::Equal => 0,
            Ordering::Greater => 1,
        }
    }

    let boxed_f: *mut F = Box::into_raw(Box::new(collation));

Review comment (bug):
I would remove the message here and downgrade this to `debug_assert!`. This is a programming error of _SQLx_. The user can't trigger this.

Code after:

unsafe extern "C" fn free_boxed_value<T>(p: *mut c_void) {
    drop(Box::from_raw(p as *mut T));
}

pub(crate) fn create_collation<F>(
    handle: &ConnectionHandle,
    name: &str,
    compare: F,
) -> Result<(), Error>
where
    F: Fn(&str, &str) -> Ordering + Send + Sync + 'static,
{
    unsafe extern "C" fn call_boxed_closure<C>(
        arg1: *mut c_void,
        arg2: c_int,
        arg3: *const c_void,
        arg4: c_int,
        arg5: *const c_void,
    ) -> c_int
    where
        C: Fn(&str, &str) -> Ordering,
    {
        let boxed_f: *mut C = arg1 as *mut C;
        debug_assert!(!boxed_f.is_null());
        let s1 = {
            let c_slice = slice::from_raw_parts(arg3 as *const u8, arg2 as usize);
            from_utf8_unchecked(c_slice)
        };
        let s2 = {
            let c_slice = slice::from_raw_parts(arg5 as *const u8, arg4 as usize);
            from_utf8_unchecked(c_slice)
        };
        let t = (*boxed_f)(s1, s2);

        match t {
            Ordering::Less => -1,
            Ordering::Equal => 0,
            Ordering::Greater => 1,
        }
    }

    let boxed_f: *mut F = Box::into_raw(Box::new(compare));
    let c_name =
        CString::new(name).map_err(|_| err_protocol!("invalid collation name: {}", name))?;
    let flags = SQLITE_UTF8;
    let r = unsafe {
        sqlite3_create_collation_v2(
            handle.as_ptr(),
            c_name.as_ptr(),
            flags,
Repository: launchbadge/sqlx
File: sqlx-core/src/sqlite/options/connect.rs (Rust)

Code before:
use crate::connection::ConnectOptions;
use crate::error::Error;
use crate::executor::Executor;
use crate::sqlite::connection::establish::establish;
use crate::sqlite::{SqliteConnectOptions, SqliteConnection};
use futures_core::future::BoxFuture;
use log::LevelFilter;
use std::time::Duration;

impl ConnectOptions for SqliteConnectOptions {
    type Connection = SqliteConnection;

    fn connect(&self) -> BoxFuture<'_, Result<Self::Connection, Error>>
    where
        Self::Connection: Sized,
    {
        Box::pin(async move {
            let mut conn = establish(self).await?;

            // send an initial sql statement comprised of options
            for (key, value) in self.pragmas.iter() {
                conn.execute(&*format!("PRAGMA {} = {};", key, value)).await?;
            }

            Ok(conn)
        })
    }

    fn log_statements(&mut self, level: LevelFilter) -> &mut Self {
        self.log_settings.log_statements(level);
        self
    }

    fn log_slow_statements(&mut self, level: LevelFilter, duration: Duration) -> &mut Self {
        self.log_settings.log_slow_statements(level, duration);
        self
    }
}


Review comment (suggestion):
Since there is a bit of overhead in `.execute()` I think it would be better if we collected the `PRAGMA` statements together in one string and executed that.

Code after:
use crate::connection::ConnectOptions;
use crate::error::Error;
use crate::executor::Executor;
use crate::sqlite::connection::establish::establish;
use crate::sqlite::{SqliteConnectOptions, SqliteConnection};
use futures_core::future::BoxFuture;
use log::LevelFilter;
use std::time::Duration;

impl ConnectOptions for SqliteConnectOptions {
    type Connection = SqliteConnection;

    fn connect(&self) -> BoxFuture<'_, Result<Self::Connection, Error>>
    where
        Self::Connection: Sized,
    {
        Box::pin(async move {
            let mut conn = establish(self).await?;

            // send an initial sql statement comprised of options
            let mut init = String::new();

            for (key, value) in self.pragmas.iter() {
                use std::fmt::Write;
                write!(init, "PRAGMA {} = {}; ", key, value).ok();
            }

            conn.execute(&*init).await?;

            Ok(conn)
        })
    }

    fn log_statements(&mut self, level: LevelFilter) -> &mut Self {
        self.log_settings.log_statements(level);
        self
    }

    fn log_slow_statements(&mut self, level: LevelFilter, duration: Duration) -> &mut Self {
        self.log_settings.log_slow_statements(level, duration);
        self
    }
}
Repository: launchbadge/sqlx
File: sqlx-core/src/sqlite/options/connect.rs (Rust)

Code before:
use crate::connection::ConnectOptions;
use crate::error::Error;
use crate::executor::Executor;
use crate::sqlite::connection::establish::establish;
use crate::sqlite::{SqliteConnectOptions, SqliteConnection};
use futures_core::future::BoxFuture;
use log::LevelFilter;
use std::time::Duration;

impl ConnectOptions for SqliteConnectOptions {
    type Connection = SqliteConnection;

    fn connect(&self) -> BoxFuture<'_, Result<Self::Connection, Error>>
    where
        Self::Connection: Sized,
    {
        Box::pin(async move {
            let mut conn = establish(self).await?;

            // send an initial sql statement comprised of options
            let mut init = String::new();

            for (key, value) in self.pragmas.iter() {
                init += &format!("PRAGMA {} = {}; ", key, value);
            }

            conn.execute(&*init).await?;

            Ok(conn)
        })
    }

    fn log_statements(&mut self, level: LevelFilter) -> &mut Self {
        self.log_settings.log_statements(level);
        self
    }

    fn log_slow_statements(&mut self, level: LevelFilter, duration: Duration) -> &mut Self {
        self.log_settings.log_slow_statements(level, duration);
        self
    }
}


Review comment (suggestion):
Add an import for `std::fmt::Write`, then:

```suggestion
               write!(init, "PRAGMA {} = {}; ", key, value).ok();
```

Code after:
use crate::connection::ConnectOptions;
use crate::error::Error;
use crate::executor::Executor;
use crate::sqlite::connection::establish::establish;
use crate::sqlite::{SqliteConnectOptions, SqliteConnection};
use futures_core::future::BoxFuture;
use log::LevelFilter;
use std::time::Duration;

impl ConnectOptions for SqliteConnectOptions {
    type Connection = SqliteConnection;

    fn connect(&self) -> BoxFuture<'_, Result<Self::Connection, Error>>
    where
        Self::Connection: Sized,
    {
        Box::pin(async move {
            let mut conn = establish(self).await?;

            // send an initial sql statement comprised of options
            let mut init = String::new();

            for (key, value) in self.pragmas.iter() {
                use std::fmt::Write;
                write!(init, "PRAGMA {} = {}; ", key, value).ok();
            }

            conn.execute(&*init).await?;

            Ok(conn)
        })
    }

    fn log_statements(&mut self, level: LevelFilter) -> &mut Self {
        self.log_settings.log_statements(level);
        self
    }

    fn log_slow_statements(&mut self, level: LevelFilter, duration: Duration) -> &mut Self {
        self.log_settings.log_slow_statements(level, duration);
        self
    }
}
Repository: launchbadge/sqlx
File: sqlx-core/src/sqlite/options/mod.rs (Rust)

Code before:
            .insert("synchronous".into(), synchronous.as_str().into());
        self
    }

    /// Sets the [auto_vacuum](https://www.sqlite.org/pragma.html#pragma_auto_vacuum) setting for the database connection.
    ///
    /// The default auto_vacuum setting is NONE.
    pub fn auto_vacuum(mut self, auto_vacuum: SqliteAutoVacuum) -> Self {
        self.pragmas
            .insert("auto_vacuum".into(), auto_vacuum.as_str().into());
        self
    }

    /// Sets the [page_size](https://www.sqlite.org/pragma.html#pragma_page_size) setting for the database connection.
    ///
    /// The default page_size setting is 4096.
    pub fn page_size(mut self, page_size: u32) -> Self {
        self.pragmas
            .insert("page_size".into(), page_size.to_string().into());
        self
    }

    /// Sets custom initial pragma for the database connection.
    pub fn pragma<T>(mut self, key: T, value: T) -> Self
    where
        T: Into<Cow<'static, str>>,
    {
        self.pragmas.insert(key.into(), value.into());
        self
    }
}


Review comment (suggestion):
```suggestion
    pub fn pragma<K, V>(mut self, key: K, value: V) -> Self
    where
        K: Into<Cow<'static, str>>,
        V: Into<Cow<'static, str>>,
```

Code after:
    pub fn synchronous(mut self, synchronous: SqliteSynchronous) -> Self {
        self.pragmas
            .insert("synchronous".into(), synchronous.as_str().into());
        self
    }

    /// Sets the [auto_vacuum](https://www.sqlite.org/pragma.html#pragma_auto_vacuum) setting for the database connection.
    ///
    /// The default auto_vacuum setting is NONE.
    pub fn auto_vacuum(mut self, auto_vacuum: SqliteAutoVacuum) -> Self {
        self.pragmas
            .insert("auto_vacuum".into(), auto_vacuum.as_str().into());
        self
    }

    /// Sets the [page_size](https://www.sqlite.org/pragma.html#pragma_page_size) setting for the database connection.
    ///
    /// The default page_size setting is 4096.
    pub fn page_size(mut self, page_size: u32) -> Self {
        self.pragmas
            .insert("page_size".into(), page_size.to_string().into());
        self
    }

    /// Sets custom initial pragma for the database connection.
    pub fn pragma<K, V>(mut self, key: K, value: V) -> Self
    where
        K: Into<Cow<'static, str>>,
        V: Into<Cow<'static, str>>,
    {
        self.pragmas.insert(key.into(), value.into());
        self
    }

    pub fn immutable(mut self, immutable: bool) -> Self {
        self.immutable = immutable;
        self
    }
  
    /// Sets the log settings.
    ///
    /// # Example
    ///
    /// ```rust
    /// # use sqlx_core::sqlite::SqliteConnectOptions;
    /// let options = SqliteConnectOptions::new()
    ///     .log_settings(sqlx_core::connection::LogSettings {
    ///     statements_level: sqlx_core::connection::LevelFilter::Info,
    ///     ..Default::default()
    /// });
    /// ```
Repository: launchbadge/sqlx
File: sqlx-core/src/sqlite/options/connect.rs (Rust)

Code before:
use crate::connection::ConnectOptions;
use crate::error::Error;
use crate::executor::Executor;
use crate::sqlite::connection::establish::establish;
use crate::sqlite::{SqliteConnectOptions, SqliteConnection};
use futures_core::future::BoxFuture;
use log::LevelFilter;
use std::time::Duration;

impl ConnectOptions for SqliteConnectOptions {
    type Connection = SqliteConnection;

    fn connect(&self) -> BoxFuture<'_, Result<Self::Connection, Error>>
    where
        Self::Connection: Sized,
    {
        Box::pin(async move {
            let mut conn = establish(self).await?;

            // send an initial sql statement comprised of options
            let mut init = String::new();

            for (key, value) in self.pragmas.iter() {
               write!(init, "PRAGMA {} = {}; ", key, value).ok();
            }

            conn.execute(&*init).await?;

            Ok(conn)
        })
    }

    fn log_statements(&mut self, level: LevelFilter) -> &mut Self {
        self.log_settings.log_statements(level);
        self
    }

    fn log_slow_statements(&mut self, level: LevelFilter, duration: Duration) -> &mut Self {
        self.log_settings.log_slow_statements(level, duration);
        self
    }
}


Review comment (suggestion):
```suggestion
               use std::fmt::Write;
               write!(init, "PRAGMA {} = {}; ", key, value).ok();
```

Code after:
use crate::connection::ConnectOptions;
use crate::error::Error;
use crate::executor::Executor;
use crate::sqlite::connection::establish::establish;
use crate::sqlite::{SqliteConnectOptions, SqliteConnection};
use futures_core::future::BoxFuture;
use log::LevelFilter;
use std::time::Duration;

impl ConnectOptions for SqliteConnectOptions {
    type Connection = SqliteConnection;

    fn connect(&self) -> BoxFuture<'_, Result<Self::Connection, Error>>
    where
        Self::Connection: Sized,
    {
        Box::pin(async move {
            let mut conn = establish(self).await?;

            // send an initial sql statement comprised of options
            let mut init = String::new();

            for (key, value) in self.pragmas.iter() {
                use std::fmt::Write;
                write!(init, "PRAGMA {} = {}; ", key, value).ok();
            }

            conn.execute(&*init).await?;

            Ok(conn)
        })
    }

    fn log_statements(&mut self, level: LevelFilter) -> &mut Self {
        self.log_settings.log_statements(level);
        self
    }

    fn log_slow_statements(&mut self, level: LevelFilter, duration: Duration) -> &mut Self {
        self.log_settings.log_slow_statements(level, duration);
        self
    }
}