Skip to content

Conversation

@Aditya4ever
Copy link

Description

Fixes TestCopyrightHeader failures on Windows environments.

The style_test.go was failing on Windows due to two issues:

  1. Line Endings: Source files checked out on Windows often have CRLF line endings (depending on git config), but the test expected exact LF matches for the copyright header.
  2. Path Separators: filepath.Walk returns paths with backslashes (\) on Windows, but the ignore map used forward slashes (/), causing ignored directories like internal/httprr to be checked erroneously.

Changes

  • Normalized file content to LF before checking for the copyright header.
  • Normalized file paths using filepath.ToSlash before checking against the ignore list.

Testing Plan

Ran go test -v ./internal on a Windows machine.

Before Fix:

--- FAIL: TestCopyrightHeader (0.11s)
    style_test.go:72: file "internal\httprr\rr.go" does not have the copyright header

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @Aditya4ever, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the robustness of the copyright header style check by introducing Windows compatibility. It specifically addresses issues related to line ending variations and path separator differences, ensuring the test functions correctly across different operating systems without producing erroneous failures.

Highlights

  • Windows Compatibility Fix: Resolved TestCopyrightHeader failures on Windows environments, which were caused by differences in line endings and path separators.
  • Line Ending Normalization: The copyright check now normalizes file content to LF line endings, accommodating Windows' CRLF line endings and preventing false negatives.
  • Path Separator Handling: Updated the path comparison logic to normalize Windows backslashes to forward slashes when checking against the ignore list, ensuring correct directory exclusion.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@google-cla
Copy link

google-cla bot commented Nov 22, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly addresses issues with the copyright header check failing on Windows environments by normalizing both file paths and line endings. The use of filepath.ToSlash is appropriate for handling path separators, and normalizing line endings for the comparison is a robust solution. I have one suggestion to improve the performance of the line ending normalization by computing the normalized header string only once, outside of the function that is called for every file.

Comment on lines +90 to +91
header := strings.ReplaceAll(copyrightHeader, "\r\n", "\n")
return strings.HasPrefix(fileContent, header), nil
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The normalization of copyrightHeader is performed on every call to hasCopyrightHeader. Since copyrightHeader is a constant, this computation is redundant. To improve performance, this can be done once at the package level.

You could define a package-level variable:

var normalizedCopyrightHeader = strings.ReplaceAll(copyrightHeader, "\r\n", "\n")

And then use it in this function, which would simplify these lines:

// ...
fileContent := strings.ReplaceAll(string(content), "\r\n", "\n")
return strings.HasPrefix(fileContent, normalizedCopyrightHeader), nil

@Aditya4ever
Copy link
Author

@googlebot I signed it.

@Aditya4ever Aditya4ever force-pushed the fix/windows-copyright-check branch from 95c46b8 to 5a123e6 Compare November 23, 2025 00:16
@Aditya4ever
Copy link
Author

@googlebot I signed it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant