ronantakizawa/codereview-bench huggingface dataset:
Repository: square/okhttp
File: okhttp/src/commonJvmAndroid/kotlin/okhttp3/Cache.kt (Kotlin)

Code before:
  )

  internal val cache =
    DiskLruCache(
      fileSystem = fileSystem,
      directory = directory,
      appVersion = VERSION,
      valueCount = ENTRY_COUNT,
      maxSize = maxSize,
      taskRunner = taskRunner,
    )

  // read and write statistics, all guarded by 'this'.
  internal var writeSuccessCount = 0
  internal var writeAbortCount = 0
  private var networkCount = 0
  private var hitCount = 0
  private var requestCount = 0

  val isClosed: Boolean
    get() = cache.isClosed()

  internal fun get(request: Request): Response? {
    if (request.body != null && request.body.isOneShot() && request.method == "QUERY") {
      // Don't cache one-shot QUERY requests, since we need to cache by using the body,
      // and we can't consume the body twice
      return null
    }
    val key = key(request)
    val snapshot: DiskLruCache.Snapshot =
      try {
        cache[key] ?: return null
      } catch (_: IOException) {
        return null // Give up because the cache cannot be read.
      }

    val entry: Entry =
      try {
        Entry(snapshot.getSource(ENTRY_METADATA))
      } catch (_: IOException) {
        snapshot.closeQuietly()
        return null
      }

    val response = entry.response(snapshot)
    if (!entry.matches(request, response)) {
      response.body.closeQuietly()
      return null
    }

    return response

Review comment (performance):
I would probably expect this to throw instead of returning `null`, both when writing and reading from the cache. Might be worth moving this check to the `key()` function?

Code after:
    directory.toOkioPath(),
    maxSize,
  )

  internal val cache =
    DiskLruCache(
      fileSystem = fileSystem,
      directory = directory,
      appVersion = VERSION,
      valueCount = ENTRY_COUNT,
      maxSize = maxSize,
      taskRunner = taskRunner,
    )

  // read and write statistics, all guarded by 'this'.
  internal var writeSuccessCount = 0
  internal var writeAbortCount = 0
  private var networkCount = 0
  private var hitCount = 0
  private var requestCount = 0

  val isClosed: Boolean
    get() = cache.isClosed()

  internal fun get(request: Request): Response? {
    if (request.body != null && request.body.isOneShot()) {
      // Don't cache one-shot QUERY requests, since we need to cache by using the body,
      // and we can't consume the body twice
      return null
    }
    val key = key(request)
    val snapshot: DiskLruCache.Snapshot =
      try {
        cache[key] ?: return null
      } catch (_: IOException) {
        return null // Give up because the cache cannot be read.
      }

    val entry: Entry =
      try {
        Entry(snapshot.getSource(ENTRY_METADATA))
      } catch (_: IOException) {
        snapshot.closeQuietly()
        return null
      }

    val response = entry.response(snapshot)
    if (!entry.matches(request, response)) {
      response.body.closeQuietly()
      return null
    }
Repository: square/okhttp
File: okhttp/src/jvmTest/kotlin/okhttp3/RequestBodyTest.kt (Kotlin)

Code before:
    }
  }

  @Test
  fun testFileDescriptorAfterClose() {
    val closedRequestBody = assertOnFileDescriptor { it.toRequestBody() }

    assertThrows(IOException::class.java) {
      closedRequestBody.writeTo(Buffer())
    }
  }

  @Test
  fun testPathRead() {
    assertOnPath(content = "Hello") { path ->
      val requestBody = path.asRequestBody(FileSystem.SYSTEM)

      val buffer = Buffer()
      requestBody.writeTo(buffer)
      assertThat(buffer.readUtf8()).isEqualTo("Hello")
    }
  }

  @Test
  fun testSha256() {
    assertOnPath(content = "Hello") { path ->
      val requestBody = path.asRequestBody(FileSystem.SYSTEM)

      assertThat(requestBody.sha256()).isEqualTo("185f8db32271fe25f561a6fc938b2e264306ec304eda518007d1764826381969")
    }
  }

  private inline fun <T> assertOnFileDescriptor(
    content: String? = null,
    fn: (FileDescriptor) -> T,
  ): T =
    assertOnPath(content) {
      FileInputStream(filePath.toFile()).use { fis ->
        fn(fis.fd)
      }
    }

  private inline fun <T> assertOnPath(
    content: String? = null,
    fn: (okio.Path) -> T,
  ): T {
    FileSystem.SYSTEM.write(filePath) {
      if (content != null) {
        writeUtf8(content)
      }
    }

Review comment (suggestion):
`assertOnPath` seems like it introduces the file system, and I don’t think we need to?

This should work just as well:

`"Hello".toRequestBody()`

Code after:
    }
  }

  @Test
  fun testFileDescriptorAfterClose() {
    val closedRequestBody = assertOnFileDescriptor { it.toRequestBody() }

    assertThrows(IOException::class.java) {
      closedRequestBody.writeTo(Buffer())
    }
  }

  @Test
  fun testPathRead() {
    assertOnPath(content = "Hello") { path ->
      val requestBody = path.asRequestBody(FileSystem.SYSTEM)

      val buffer = Buffer()
      requestBody.writeTo(buffer)
      assertThat(buffer.readUtf8()).isEqualTo("Hello")
    }
  }

  @Test
  fun testSha256() {
    val hash = "Hello".toRequestBody().sha256().hex()
    assertThat(hash).isEqualTo("185f8db32271fe25f561a6fc938b2e264306ec304eda518007d1764826381969")
  }

  private inline fun <T> assertOnFileDescriptor(
    content: String? = null,
    fn: (FileDescriptor) -> T,
  ): T =
    assertOnPath(content) {
      FileInputStream(filePath.toFile()).use { fis ->
        fn(fis.fd)
      }
    }

  private inline fun <T> assertOnPath(
    content: String? = null,
    fn: (okio.Path) -> T,
  ): T {
    FileSystem.SYSTEM.write(filePath) {
      if (content != null) {
        writeUtf8(content)
      }
    }

    return fn(filePath)
  }
Repository: square/okhttp
File: okhttp/src/jvmTest/kotlin/okhttp3/internal/http/HttpUpgradesTest.kt (Kotlin)

Code before:
  @Test
  fun upgradeEventsWithoutRequestBody() {
    upgrade()

    assertThat(listener.recordedEventTypes()).containsExactly(
      CallStart::class,
      ProxySelectStart::class,
      ProxySelectEnd::class,
      DnsStart::class,
      DnsEnd::class,
      ConnectStart::class,
      ConnectEnd::class,
      ConnectionAcquired::class,
      RequestHeadersStart::class,
      RequestHeadersEnd::class,
//      RequestBodyStart::class,
//      RequestBodyEnd::class,
      ResponseHeadersStart::class,
      ResponseHeadersEnd::class,
      FollowUpDecision::class,
      RequestBodyStart::class,
      ResponseBodyStart::class,
      ResponseBodyEnd::class,
      ConnectionReleased::class,
      CallEnd::class,
      RequestBodyEnd::class,
    )
  }

  @Test
  fun upgradeEventsWithEmptyRequestBody() {
    upgradeWithEmptyRequestBody()

    assertThat(listener.recordedEventTypes()).containsExactly(
      CallStart::class,
      ProxySelectStart::class,
      ProxySelectEnd::class,
      DnsStart::class,
      DnsEnd::class,
      ConnectStart::class,
      ConnectEnd::class,
      ConnectionAcquired::class,
      RequestHeadersStart::class,
      RequestHeadersEnd::class,
      RequestBodyStart::class,
      RequestBodyEnd::class,
      ResponseHeadersStart::class,
      ResponseHeadersEnd::class,
      FollowUpDecision::class,
      RequestBodyStart::class,
      ResponseBodyStart::class,

Review comment (suggestion):
I'm not sure how to let the `RequestBodyEnd` appear before `CallEnd` or before `ConnectionReleased`.

Code after:

  @Test
  fun upgradeEventsWithoutRequestBody() {
    upgrade()

    assertThat(listener.recordedEventTypes()).containsExactly(
      CallStart::class,
      ProxySelectStart::class,
      ProxySelectEnd::class,
      DnsStart::class,
      DnsEnd::class,
      ConnectStart::class,
      ConnectEnd::class,
      ConnectionAcquired::class,
      RequestHeadersStart::class,
      RequestHeadersEnd::class,
      ResponseHeadersStart::class,
      ResponseHeadersEnd::class,
      FollowUpDecision::class,
      RequestBodyStart::class,
      ResponseBodyStart::class,
      ResponseBodyEnd::class,
      RequestBodyEnd::class,
      ConnectionReleased::class,
      CallEnd::class,
    )
  }

  @Test
  fun upgradeEventsWithEmptyRequestBody() {
    upgradeWithEmptyRequestBody()

    assertThat(listener.recordedEventTypes()).containsExactly(
      CallStart::class,
      ProxySelectStart::class,
      ProxySelectEnd::class,
      DnsStart::class,
      DnsEnd::class,
      ConnectStart::class,
      ConnectEnd::class,
      ConnectionAcquired::class,
      RequestHeadersStart::class,
      RequestHeadersEnd::class,
      RequestBodyStart::class,
      RequestBodyEnd::class,
      ResponseHeadersStart::class,
      ResponseHeadersEnd::class,
      FollowUpDecision::class,
      RequestBodyStart::class,
      ResponseBodyStart::class,
      ResponseBodyEnd::class,
Repository: square/okhttp
File: android-test-app/build.gradle.kts (Kotlin)

Code before:
@file:Suppress("UnstableApiUsage")

plugins {
  id("com.android.application")
  id("kotlin-android")
}

android {
  compileSdk = 36

  namespace = "okhttp.android.testapp"

  testBuildType = "release"

  defaultConfig {
    minSdk = 21
    targetSdk = 36
    testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
  }

  compileOptions {
    targetCompatibility(JavaVersion.VERSION_11)
    sourceCompatibility(JavaVersion.VERSION_11)
  }

  kotlinOptions {
    jvmTarget = JavaVersion.VERSION_11.toString()
  }

  buildTypes {
    release {
      isShrinkResources = true
      isMinifyEnabled = true
      signingConfig = signingConfigs.getByName("debug")
      setProguardFiles(listOf(getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro"))
      testProguardFiles("test-proguard-rules.pro")
    }
  }

  lint {
    abortOnError = true
  }
}

dependencies {
  implementation(libs.playservices.safetynet)
  implementation(projects.okhttp)
  implementation(libs.androidx.activity)

  androidTestImplementation(libs.androidx.junit)
  androidTestImplementation(libs.androidx.espresso.core)

Review comment (suggestion):
I need to confirm this is just because AGP is now 17+, but the artifacts still work for older android clients...

Code after:
@file:Suppress("UnstableApiUsage")

plugins {
  id("com.android.application")
}

android {
  compileSdk = 36

  namespace = "okhttp.android.testapp"

  testBuildType = "release"

  defaultConfig {
    minSdk = 21
    targetSdk = 36
    testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
  }

  compileOptions {
    targetCompatibility(JavaVersion.VERSION_11)
    sourceCompatibility(JavaVersion.VERSION_11)
  }


  buildTypes {
    release {
      isShrinkResources = true
      isMinifyEnabled = true
      signingConfig = signingConfigs.getByName("debug")
      setProguardFiles(listOf(getDefaultProguardFile("proguard-android-optimize.txt"), "proguard-rules.pro"))
      testProguardFiles("test-proguard-rules.pro")
    }
  }

  lint {
    abortOnError = true
  }
}

dependencies {
  implementation(libs.playservices.safetynet)
  implementation(projects.okhttp)
  implementation(libs.androidx.activity)

  androidTestImplementation(libs.androidx.junit)
  androidTestImplementation(libs.androidx.espresso.core)
  androidTestImplementation(libs.androidx.test.runner)
  androidTestImplementation(libs.assertk)
}
Repository: square/okhttp
File: build-logic/src/main/kotlin/okhttp.jvm-conventions.gradle.kts (Kotlin)

Code before:

configureKotlinCompatibility(version = version("kotlinCoreLibrariesVersion"))

tasks.withType<JavaCompile> {
  options.encoding = Charsets.UTF_8.toString()
  if (name.contains("Java9")) {
    sourceCompatibility = "9"
    targetCompatibility = "9"
  } else {
    sourceCompatibility = "1.8"
    targetCompatibility = "1.8"
  }
}

tasks.withType<KotlinCompile> {
  compilerOptions {
    jvmTarget.set(JvmTarget.JVM_1_8)
    freeCompilerArgs.addAll(
      "-Xjvm-default=all",
      "-Xexpect-actual-classes",
    )
  }
}

extensions.configure<TapmocExtension> {
  // Fail the build if any api dependency exposes incompatible Kotlin metadata, Kotlin stdlib or Java bytecode version.
  checkDependencies()
}


Review comment (bug):
```suggestion
  // Fail the build if any api dependency exposes incompatible Kotlin metadata, Kotlin stdlib, or Java bytecode version.
```
One of my hills...

Code after:

configureKotlinCompatibility(version = version("kotlinCoreLibrariesVersion"))

tasks.withType<JavaCompile> {
  options.encoding = Charsets.UTF_8.toString()
  if (name.contains("Java9")) {
    sourceCompatibility = "9"
    targetCompatibility = "9"
  } else {
    sourceCompatibility = "1.8"
    targetCompatibility = "1.8"
  }
}

tasks.withType<KotlinCompile> {
  compilerOptions {
    jvmTarget.set(JvmTarget.JVM_1_8)
    freeCompilerArgs.addAll(
      "-Xjvm-default=all",
      "-Xexpect-actual-classes",
    )
  }
}

extensions.configure<TapmocExtension> {
  // Fail the build if any api dependency exposes incompatible Kotlin metadata, Kotlin stdlib, or Java bytecode version.
  checkDependencies()
}
Repository: srbhr/Resume-Matcher
File: webapp/app/_components/third-party-services/reducer.ts (TypeScript)

Code before:
  payload: boolean;
};

export type ReducerAction = AddServiceKeyAction | UpdateButtonStateAction;

export function reducer(state: ReducerState, action: ReducerAction) {
  switch (action.type) {
    case "ADD_SERVICE_KEY": {
      return {
        ...state,
        serviceKeys: {
          ...state.serviceKeys,
          ...action.payload,
        },
      };
    }
    case "UPDATE_BUTTON_STATE": {
      return {
        ...state,
        shouldDisableButtons: action.payload,
      };
    }
  }

  return state;
}

Review comment (suggestion):
I think you need to update a VS Code IDE setting to automatically append a new line (on save) at the end of files, otherwise GitHub Pull Requests will flag this up like so as an unnecessary change.

It's common practise nowadays.

Code after:
  payload: boolean;
};

export type ReducerAction = AddServiceKeyAction | UpdateButtonStateAction;

export function reducer(state: ReducerState, action: ReducerAction) {
  switch (action.type) {
    case "ADD_SERVICE_KEY": {
      return {
        ...state,
        serviceKeys: {
          ...state.serviceKeys,
          ...action.payload,
        },
      };
    }
    case "UPDATE_BUTTON_STATE": {
      return {
        ...state,
        shouldDisableButtons: action.payload,
      };
    }
  }

  return state;
}
Repository: srbhr/Resume-Matcher
File: webapp/app/(routes)/page.tsx (TypeScript)

Code before:
import JobDescriptions from "@/components/job-descriptions/job-descriptions";
import VectorScore from "@/components/vector-scores/vector-scores";
import CommonWords from "@/components/common-words/common-words";
import Suggestions from "@/components/suggestions/suggestions";
import ThirdPartyServicesKeys from "@/components/third-party-services/third-party-services";
import FileUpload from "@/components/resume/file-upload/file-upload";
import ResumeGlance from "@/components/resume/resume-glance/resume-glance";
import Hero from "@/components/hero/hero";
import Footer from "../_components/footer/footer";

export default function Home() {
  return (
    <main className="text-[white]">
      <Hero>
        <ThirdPartyServicesKeys />
        <FileUpload buttonLabel="Upload Your Resume" />
      </Hero>
      <ResumeGlance />
      <JobDescriptions />
      <VectorScore />
      <CommonWords />
      <Suggestions />
      <Footer />
    </main>
  );
}


Review comment (suggestion):
👀  There's no need to wrap the pre-defined colour within square brackets.

Instead, we I suggest we simply leverage the existing baked-in Tailwind CSS class `text-white`, which is also consistently used elsewhere throughout the web app codebase.

```suggestion
    <main className="text-white">
```

Code after:
import JobDescriptions from "@/components/job-descriptions/job-descriptions";
import VectorScore from "@/components/vector-scores/vector-scores";
import CommonWords from "@/components/common-words/common-words";
import Suggestions from "@/components/suggestions/suggestions";
import ThirdPartyServicesKeys from "@/components/third-party-services/third-party-services";
import FileUpload from "@/components/resume/file-upload/file-upload";
import ResumeGlance from "@/components/resume/resume-glance/resume-glance";
import Hero from "@/components/hero/hero";
import Footer from "../_components/footer/footer";

export default function Home() {
  return (
    <main className="text-white">
      <Hero>
        <ThirdPartyServicesKeys />
        <FileUpload buttonLabel="Upload Your Resume" />
      </Hero>
      <ResumeGlance />
      <JobDescriptions />
      <VectorScore />
      <CommonWords />
      <Suggestions />
      <Footer />
    </main>
  );
}
Repository: srbhr/Resume-Matcher
File: webapp/app/_components/footer/footer.tsx (TypeScript)

Code before:
import Image from "next/image";
import data from "./footerLinks";

const Footer = () => {
  return (
    <>
      <section className="text-[#CFCDD2] bg-[#221932] px-24 py-16">
        <div className="flex justify-between ">
          <div className="text-3xl w-2/5 ">
            <p className="pb-4 font-semibold">Resume Matcher</p>
            <ul className="flex ">
              <a
                className="text-base pr-4 hover:text-[#A49DA2]"
                href="https://github.com/srbhr/Resume-Matcher"
                target="_blank"
                rel="noopener noreferrer"
              >
                Github
              </a>
              <a
                className="text-base hover:text-[#A49DA2]"
                href="https://github.com/srbhr/Resume-Matcher/blob/master/LICENSE"
                target="_blank"
                rel="noopener noreferrer"
              >
                Apache 2.0
              </a>
            </ul>
          </div>
          <div className="flex justify-around text-xl w-3/5">
            {data.map((section, index) => (

Review comment (suggestion):
💡 Optional: We don't necessarily need to use/ wrap with the React fragment (`<>`) since the `<section>` element is the single parent element that is returned from this React component.

Code after:
import Image from "next/image";
import data from "./footerLinks";

const Footer = () => {
  return (
    <section className="text-[#CFCDD2] bg-[#221932] px-24 py-16">
      <div className="flex justify-between">
        <div className="text-3xl w-2/5">
          <p className="pb-4 font-semibold">Resume Matcher</p>
          <ul className="flex">
            <a
              className="text-base pr-4 hover:text-[#A49DA2]"
              href="https://github.com/srbhr/Resume-Matcher"
              target="_blank"
              rel="noopener noreferrer"
            >
              Github
            </a>
            <a
              className="text-base hover:text-[#A49DA2]"
              href="https://github.com/srbhr/Resume-Matcher/blob/master/LICENSE"
              target="_blank"
              rel="noopener noreferrer"
            >
              Apache 2.0
            </a>
          </ul>
        </div>
        <div className="flex justify-around text-xl w-3/5">
          {data.map((section, index) => (
            <div key={index}>
Repository: srbhr/Resume-Matcher
File: webapp/app/_components/footer/footer.tsx (TypeScript)

Code before:
import Image from "next/image";
import data from "./footerLinks";

const Footer = () => {
  return (
    <>
      <section className="text-[#CFCDD2] bg-[#221932] px-24 py-16">
        <div className="flex justify-between ">
          <div className="text-3xl w-2/5 ">
            <p className="pb-4 font-semibold">Resume Matcher</p>
            <ul className="flex ">
              <a
                className="text-base pr-4 hover:text-[#A49DA2]"
                href="https://github.com/srbhr/Resume-Matcher"
                target="_blank"
                rel="noopener noreferrer"
              >
                Github
              </a>
              <a
                className="text-base hover:text-[#A49DA2]"
                href="https://github.com/srbhr/Resume-Matcher/blob/master/LICENSE"
                target="_blank"
                rel="noopener noreferrer"
              >
                Apache 2.0
              </a>
            </ul>
          </div>
          <div className="flex justify-around text-xl w-3/5">
            {data.map((section, index) => (
              <div key={index}>
                <p className="pb-4 font-semibold">{section.label}</p>
                <ul>
                  {section.items.map((item, itemIndex) => (
                    <li

Review comment (suggestion):
💡 Optional: We could remove the additional space for consistent and proper formatting, and even cuts down on the bundled byte size (negligible)

```suggestion
        <div className="flex justify-between">
          <div className="text-3xl w-2/5">
            <p className="pb-4 font-semibold">Resume Matcher</p>
            <ul className="flex">
```

Code after:
import Image from "next/image";
import data from "./footerLinks";

const Footer = () => {
  return (
    <section className="text-[#CFCDD2] bg-[#221932] px-24 py-16">
      <div className="flex justify-between">
        <div className="text-3xl w-2/5">
          <p className="pb-4 font-semibold">Resume Matcher</p>
          <ul className="flex">
            <a
              className="text-base pr-4 hover:text-[#A49DA2]"
              href="https://github.com/srbhr/Resume-Matcher"
              target="_blank"
              rel="noopener noreferrer"
            >
              Github
            </a>
            <a
              className="text-base hover:text-[#A49DA2]"
              href="https://github.com/srbhr/Resume-Matcher/blob/master/LICENSE"
              target="_blank"
              rel="noopener noreferrer"
            >
              Apache 2.0
            </a>
          </ul>
        </div>
        <div className="flex justify-around text-xl w-3/5">
          {data.map((section, index) => (
            <div key={index}>
              <p className="pb-4 font-semibold">{section.label}</p>
              <ul>
                {section.items.map((item, itemIndex) => (
                  <li
                    className="py-1 text-base hover:text-[#A49DA2]"