Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libfetchers-tests: Add back git-utils.cc #12511

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

xokdvium
Copy link
Contributor

@xokdvium xokdvium commented Feb 18, 2025

Motivation

Seems like this got dropped at some point during meson migration, so
put it back in the build system.

Drop all tests for parseGitUrl, since that function doesn't exist
and migrating doesn't look sensible because git-lfs stuff seems to use
ParsedURL.

Context


Add 👍 to pull requests you find important.

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

@xokdvium xokdvium requested a review from edolstra as a code owner February 18, 2025 20:43
@Ericson2314
Copy link
Member

@xokdvium I think it would be better to fix this. In the old build system, we didn't need to reference all the build files. The Meson rewrite was done over the better part of a year, so it was sadly easy to forget files.

Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

It would be better to restore these tests, unless that is not possible.

Seems like this got dropped at some point during meson migration, so
put it back in the build system.

Drop all tests for `parseGitUrl`, since that function doesn't exist
and migrating doesn't look sensible because git-lfs stuff seems to use
`ParsedURL`.
@xokdvium xokdvium force-pushed the chore/delete-dead-code branch from db643c5 to d95b7fe Compare February 20, 2025 20:20
@xokdvium xokdvium changed the title Remove unused libfetchers-tests/git-utils.cc libfetchers-tests: Add back git-utils.cc Feb 20, 2025
@xokdvium xokdvium requested a review from Ericson2314 February 20, 2025 20:20
@Ericson2314
Copy link
Member

Yeah I guess so

 $ git log -G '\<GitUrl\>'
commit 2a2518b4083e9d583aa53ee2d0bae3072a3e2147
Author: Leandro Reina <[email protected]>
Date:   Fri Jan 10 18:32:09 2025 +0100

    LFS code review

commit 70ffcc83d7aed2613f4cc546af0b9b28d05bfdd8
Author: Leandro Reina <[email protected]>
Date:   Wed Nov 20 18:24:17 2024 +0100

    Fix format

commit b69fb151c453d816f943b883b880c19bdd6bcb22
Author: Brian Camacho <[email protected]>
Date:   Sun Nov 10 03:41:05 2024 -0500

    better url handling; unit tests

@Ericson2314 Ericson2314 merged commit 61f49de into NixOS:master Feb 20, 2025
14 checks passed
@xokdvium xokdvium deleted the chore/delete-dead-code branch February 20, 2025 20:58
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.

3 participants