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

Feature: Support Reading Filter Expressions from a File #1411

Merged
merged 7 commits into from
Dec 21, 2024

Conversation

schoenherrg
Copy link
Contributor

@schoenherrg schoenherrg commented Dec 20, 2024

Fixes #1409

Requirements

All new code should be covered with tests, documentation should be updated. CI should pass.

Description of the Change

As mentioned in the Issue, we have filter expressions that exceed the command line length limit and thus need to be read from a file/stdin. This PR extends the Aptly command-line interface to be able to read filter expressions from a file using the @/path/to/file syntax or @- for stdin.

Our use case is only for the -filter argument for mirror create/mirror edit. However, there are other places in Aptly where filter expressions are accepted, so I added support for the @file syntax everywhere for consistency.

Commands where the @file syntax is supported now:

  • mirror create
  • mirror edit
  • package search
  • package show
  • repo move
  • repo remove
  • snapshot filter
  • snapshot pull
  • snapshot search

Checklist

  • unit-test added (if change is algorithm)
  • functional test added/updated (if change is functional)
  • man page updated (if applicable)
  • bash completion updated (if applicable)
  • documentation updated
  • author name in AUTHORS

@schoenherrg schoenherrg marked this pull request as draft December 20, 2024 01:10
@schoenherrg schoenherrg force-pushed the feature/filter-using-file branch from d8e45a8 to 8830354 Compare December 20, 2024 02:00
@schoenherrg schoenherrg force-pushed the feature/filter-using-file branch from cab97a6 to 50d3676 Compare December 20, 2024 03:56
@schoenherrg schoenherrg marked this pull request as ready for review December 20, 2024 05:11
@neolynx neolynx added the needs review Ready for review & merge label Dec 20, 2024
@neolynx neolynx requested a review from a team December 20, 2024 10:11
@neolynx
Copy link
Member

neolynx commented Dec 20, 2024

excellent feature 👍

@neolynx
Copy link
Member

neolynx commented Dec 20, 2024

would this also fix #157 ?

@neolynx
Copy link
Member

neolynx commented Dec 20, 2024

we should increase the coverage a bit...

codecov/patch Failing after 1s — 60.49% of diff hit (target 75.09%)

@neolynx
Copy link
Member

neolynx commented Dec 20, 2024

@5hir0kur0 please push the branch to the main aptly-dev/aptly git, then the coverage runs automatically. otherwise I need to do a separate coverage pr.. :) could you open the PR there ?

@neolynx neolynx added the increase coverage The PR lacks test coverage label Dec 20, 2024
cmd/mirror_create.go Outdated Show resolved Hide resolved
@neolynx
Copy link
Member

neolynx commented Dec 21, 2024

the coverage test does not seem to cover all of the code, when it is invoked by the flag parsing.
I also tryed adding a test with a non existint @file, but was unable to get coverage output, as the program is aborted on error.

let's accept low coverage here...

@neolynx neolynx removed the increase coverage The PR lacks test coverage label Dec 21, 2024
@neolynx neolynx requested a review from a team December 21, 2024 17:53
@neolynx neolynx merged commit e5b8315 into aptly-dev:master Dec 21, 2024
127 of 128 checks passed
@schoenherrg
Copy link
Contributor Author

@neolynx Thanks for your work!

@5hir0kur0 please push the branch to the main aptly-dev/aptly git, then the coverage runs automatically. otherwise I need to do a separate coverage pr.. :) could you open the PR there?

Sorry, I didn't check my GitHub over the weekend, but I'll do it like that next time I make a PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Ready for review & merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Suggestion: Support Reading Filter Expressions from a File
3 participants