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

make: generate auto-completion files #118

Merged
merged 3 commits into from
Jan 28, 2024

Conversation

rst0git
Copy link
Member

@rst0git rst0git commented Jan 26, 2024

This pull request extends the Makefile with targets to generate, install, and uninstall auto-completion files.

We currently have a single line that contains the declarations for all
phony targets. As we add new makefile targets, this line becomes
increasingly long. Using declarations on separate lines would be easier
to maintain.

Signed-off-by: Radostin Stoyanov <[email protected]>
@rst0git rst0git requested a review from adrianreber January 26, 2024 11:14
Copy link

github-actions bot commented Jan 26, 2024

Test Results

49 tests  ±0   49 ✅ ±0   1s ⏱️ ±0s
 1 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit b76a390. ± Comparison against base commit d132ca0.

♻️ This comment has been updated with latest results.

@codecov-commenter
Copy link

codecov-commenter commented Jan 26, 2024

Codecov Report

Attention: 102 lines in your changes are missing coverage. Please review.

Comparison is base (9aa0386) 78.85% compared to head (b76a390) 78.67%.
Report is 13 commits behind head on main.

Files Patch % Lines
internal/json.go 66.66% 58 Missing and 20 partials ⚠️
internal/utils.go 66.00% 12 Missing and 5 partials ⚠️
cmd/memparse.go 90.41% 4 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #118      +/-   ##
==========================================
- Coverage   78.85%   78.67%   -0.19%     
==========================================
  Files           6        9       +3     
  Lines         927     1158     +231     
==========================================
+ Hits          731      911     +180     
- Misses        154      185      +31     
- Partials       42       62      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@adrianreber
Copy link
Member

Do we need to keep the generated files in the repository?

@rst0git
Copy link
Member Author

rst0git commented Jan 26, 2024

Do we need to keep the generated files in the repository?

I used an approach similar to Podman (https://github.com/containers/podman/tree/main/completions); it simplifies the installation process. With this approach we don't need to generate the auto-completion files every time we build/install checkpointctl, but we can use make completions to update them.

@rst0git rst0git force-pushed the completions branch 2 times, most recently from 73ab76b to eb04e19 Compare January 26, 2024 11:48
@adrianreber
Copy link
Member

I kind of like it to have information about the completion in the main README file. No need from my side to move it somewhere else. Besides that, this looks good.

This patch extends the Makefile with targets to generate, install,
and uninstall auto-completion files.

Signed-off-by: Radostin Stoyanov <[email protected]>
Signed-off-by: Radostin Stoyanov <[email protected]>
@rst0git rst0git merged commit 3eefc30 into checkpoint-restore:main Jan 28, 2024
10 checks passed
@rst0git rst0git deleted the completions branch January 28, 2024 19:36
rst0git added a commit to rst0git/checkpointctl that referenced this pull request Jun 10, 2024
Major highlights of this release:

- Added annotations for creating OCI images (checkpoint-restore#127)
- Added support for list command (checkpoint-restore#115)
- Added auto-completion files (checkpoint-restore#118)

Signed-off-by: Radostin Stoyanov <[email protected]>
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