Skip to content

Refactoring, and a usage tweak, for 2580 #2

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Refactoring, and a usage tweak, for 2580 #2

wants to merge 2 commits into from

Conversation

@edsantiago
Copy link

@edsantiago edsantiago commented Mar 7, 2019

Add a badSubArg function to common.go, allowing for less duplication in each function that takes subcommands.

Tweak the Usage template to avoid multiple usage lines (one with [flags], one with [command]). Display just [command] when appropriate.

Signed-off-by: Ed Santiago santiago@redhat.com

@edsantiago
Copy link
Author

edsantiago commented Mar 7, 2019

DANGER! DO NOT ACCEPT AS-IS!

My badSubArg throws index out of range if called with no subcommand (as opposed to an unknown one). Totally my bad. I'll leave a fix to you, should you choose to accept my amendment.

@rhatdan
More cleanup for failures on missing commands.
0661104 
Currently in podman if a user specifies a command that does not exist
the tool shows the help information.  This patch changes it to show
information like:

$ ./bin/podman foobar
Error: unrecognized command 'podman foobar'
Try 'podman --help' for more information.
$ ./bin/podman volume foobar
Error: unrecognized command `podman volume foobar`
Try 'podman volume --help' for more information.
$ ./bin/podman container foobar
Error: unrecognized command `podman container foobar`
Try 'podman container --help' for more information.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@edsantiago
Refactoring of 2580
8661fe0 
Define one central helper function that can be used by any
podman command taking subcommands; it will complain on
invalid or missing subcommand.

Signed-off-by: Ed Santiago <santiago@redhat.com>
@edsantiago
Copy link
Author

edsantiago commented Mar 8, 2019

New force-push: rebased against your rebase, fixed the illegal access, added checkin comment.

@rhatdan for best results, diff against master (94e89fc) not against yours. That will give a better indication of the cleanup.

@edsantiago
Copy link
Author

edsantiago commented Mar 8, 2019

Ooh, there's a way to do it from github! Try this diff against master. I hope you like it.

@rhatdan rhatdan force-pushed the cleanup1 branch 2 times, most recently from d99fe0b to 181f327 Compare March 8, 2019 12:06
@rhatdan
Copy link
Owner

rhatdan commented Mar 8, 2019

@edsantiago I stole your excellent idea. Thanks for figuring it out.

@rhatdan rhatdan closed this Mar 8, 2019
@edsantiago edsantiago deleted the pr2580_refactored branch March 11, 2019 14:04
rhatdan pushed a commit that referenced this pull request Mar 13, 2019
@TomSweeneyRedHat
Vendor docker/docker, fsouza and more #2
8f418f1 
Signed-off-by: TomSweeneyRedHat <tsweeney@redhat.com>

Vendors in fsouza/docker-client, docker/docker and
a few more related. Of particular note, changes to the TweakCapabilities()
function from docker/docker along with the parse.IDMappingOptions() function
from Buildah. Please pay particular attention to the related changes in
the call from libpod to those functions during the review.

Passes baseline tests.
rhatdan pushed a commit that referenced this pull request Mar 13, 2019
@openshift-merge-robot
Merge pull request containers#2628 from TomSweeneyRedHat/dev/tsweeney…
1fa6641 
…/bigvendorbuildah2

Vendor docker/docker, fsouza and more #2
rhatdan pushed a commit that referenced this pull request Jul 11, 2019
@cevich
Cirrus: Automate releasing of tested binaries
1ef8637 
It's desirable to make archives available of builds containing actual
tested content.  While not official distro-releases, these will enable
third-party testing, experimentation, and development for both branches
(e.g. "master") and pull requests (e.g. "pr3106").

* Add a Makefile targets for archiving both regular podman binaries
  and the remote-client.  Encode release metadata within these
  archives so that their exact source can be identified.

* Fix bug with cross-compiling remote clients for the Windows and Darwin
  platforms.

* Add unit-testing of cross-compiles for Windows and Darwin platforms.

* A few small CI-script typo-fixes

* Add a script which operates in two modes:

  1. Call Makefile targets which produce release archives.
     Upload the archive to Cirrus-CI's built-in caching system
     using reproducible cache keys.

  2. Utilize reproduced cache keys to attempt download of cache
     from each tasks.  When successful, parse the file's
     release metadata, using it to name the archive file.  Upload
     all recovered archives to a publicly accessible storage bucket
     for future reference.

* Update the main testing task to call the script in mode #1 for
  all primary platforms.

* Add a new `$SPECIALMODE` task to call the script in mode #1 for
  Windows and Darwin targets.

* Add a new 'release' task to the CI system, dependent upon all other
  tasks.  This new tasks executes the script in mode #2.

* Update CI documentation

Signed-off-by: Chris Evich <cevich@redhat.com>
rhatdan pushed a commit that referenced this pull request Jan 24, 2020
@baude
Review corrections pass #2
cf7be58 
Add API review comments to correct documentation and endpoints.  Also, add a libpode prune method to reduce code duplication.  Only used right now for the API but when the remote client is wired, we will switch over there too.

Signed-off-by: Brent Baude <bbaude@redhat.com>
rhatdan pushed a commit that referenced this pull request Jan 24, 2020
@openshift-merge-robot
Merge pull request containers#4953 from baude/reviewcorrections2
5bad873 
Review corrections pass #2
rhatdan pushed a commit that referenced this pull request Feb 18, 2020
@edsantiago
Swagger: fix inconsistencies (try #2)
2a411bc 
As I've mentioned once or twice, hand-maintained swagger docs
are evil. This commit attempts to fix:

  * Inconsistent methods (swagger says POST but code signature
    says GET)

  * Inconsistent capitalization

  * Typos ("Mounter", "pood")

  * Completely wrong paths (/inspect vs /json)

  * Missing .Method() registrations

  * Missing /libpod in some /volumes paths

  * Incorrect method declaration: /libpod/containers/.../kill
    was correct (POST) in swagger but wrong in the code itself
    (http.MethodGet). Correct the latter to MethodPost

This is two hours' work, even with a script I have that
tries to cross-check everything.

Swagger docs should not be human-maintained.

Signed-off-by: Ed Santiago <santiago@redhat.com>
rhatdan pushed a commit that referenced this pull request Feb 18, 2020
@openshift-merge-robot
Merge pull request containers#5231 from edsantiago/swagger_fixes
931eb1b 
Swagger: fix inconsistencies (try #2)
snj33v pushed a commit to snj33v/libpod that referenced this pull request May 31, 2020
@edsantiago @snj33v
Swagger: fix inconsistencies (try rhatdan#2)
5380970 
As I've mentioned once or twice, hand-maintained swagger docs
are evil. This commit attempts to fix:

  * Inconsistent methods (swagger says POST but code signature
    says GET)

  * Inconsistent capitalization

  * Typos ("Mounter", "pood")

  * Completely wrong paths (/inspect vs /json)

  * Missing .Method() registrations

  * Missing /libpod in some /volumes paths

  * Incorrect method declaration: /libpod/containers/.../kill
    was correct (POST) in swagger but wrong in the code itself
    (http.MethodGet). Correct the latter to MethodPost

This is two hours' work, even with a script I have that
tries to cross-check everything.

Swagger docs should not be human-maintained.

Signed-off-by: Ed Santiago <santiago@redhat.com>
rhatdan pushed a commit that referenced this pull request Jun 29, 2020
@vrothberg
Merge pull request #2 from edsantiago/pr6751-test
279ccbd 
Friendly amendment for pr 6751
rhatdan added a commit that referenced this pull request Jan 21, 2022
@rhatdan
Fix #2 for compat commit handling of --changes
653da8f 
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
rhatdan pushed a commit that referenced this pull request Jan 21, 2022
@openshift-merge-robot
Merge pull request containers#12951 from rhatdan/commit
e3ea996 
Fix #2 for compat commit handling of --changes
rhatdan pushed a commit that referenced this pull request Jul 11, 2023
@baude
AppleHV enablement pass #2
4e96686 
* Use vfkit command line assembly
* Inject ignition file into guest using http over vsock
* Ready notification through use of vsock

[NO NEW TESTS NEEDED]

Signed-off-by: Brent Baude <bbaude@redhat.com>
rhatdan pushed a commit that referenced this pull request Jul 11, 2023
@openshift-merge-robot
Merge pull request containers#19131 from baude/applehvpass2
b994f86 
AppleHV enablement pass #2
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

locked - please file new issue/PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@edsantiago @rhatdan