Skip to content

Conversation

@Zaczero
Copy link
Member

@Zaczero Zaczero commented Jan 6, 2026

Motivation

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@Zaczero Zaczero requested a review from Ericson2314 as a code owner January 6, 2026 17:09
@github-actions github-actions bot added the store Issues and pull requests concerning the Nix store label Jan 6, 2026
Comment on lines +112 to +114
static constexpr std::string_view appContents = ".app/Contents/";
auto pos = path.find(appContents);
if (pos != std::string::npos && pos + appContents.size() < path.size()) {
Copy link
Member

@edolstra edolstra Jan 6, 2026

Choose a reason for hiding this comment

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

What is the condition appContents.size() < path.size() for? Can't this just be written as

Suggested change
static constexpr std::string_view appContents = ".app/Contents/";
auto pos = path.find(appContents);
if (pos != std::string::npos && pos + appContents.size() < path.size()) {
if (path.find(".app/Contents/") != path.end()) {

Copy link
Member Author

Choose a reason for hiding this comment

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

because .+$ assumes there must be at least one more char and I preferred to be very strict about any behavioral changes

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

Labels

store Issues and pull requests concerning the Nix store

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants