Start exposing lists and package results via the API #132 #137
Conversation
|
@ferventcoder I gave the Travis build a kick, as it seemed to time out when restoring nuget packages |
|
Let me know what you think -- I have at least one more "list" method to add (on the list command) and probably two (on the install command). Also -- I stuck your copyright statement on the tops of the three files I had to add, but I noticed all your copyright headers say 2011 (except the three that I just added, ha). |
They say 2011 - Present. |
|
Let's talk about your commit messages - https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md#prepare-commits |
| Value = source.Value, | ||
| Disabled = source.Disabled, | ||
| Authenticated = string.IsNullOrWhiteSpace(source.Password) | ||
| }; |
There was a problem hiding this comment.
One can logresults AND return, which could get interesting if you want it to log all of the results before returning. What benefits do you gain in yielding?
There was a problem hiding this comment.
Nothing much -- I'm outputting the members as I receive them. It's a habit from writing PowerShell pipeline stuff, and it also means I don't have to actually create an IEnumerable to return.
It's fine to cache the new objects and output them all at once if you prefer.
|
So much prettier |
|
Okay, you can rebase and drop 524f217 |
|
|
||
| namespace chocolatey.infrastructure.app.configuration | ||
| { | ||
| public sealed class ChocolateySource |
There was a problem hiding this comment.
Since this is presented to the outside, would you think sealed or inheritable?
There was a problem hiding this comment.
sure, I'll unseal it.
In order to make commands support listing, add a new command interface the IListCommand interface is an ICommand with an extra method "list"
Refactor GenericRunner.run to a common find_command and separate the run and list methods which call different methods on the underlying command (IListCommand). Previously we only supported GenericRunner.run, this also updates GetChocolatey to add a list method that calls GenericRunner.list
|
Do you want to merge this one and deal with the others separately? |
| configuration.RegularOuptut = true; | ||
| var runner = new GenericRunner(); | ||
| return runner.list<T>(configuration, _container, isConsole: false, parseArgs: null); | ||
| } |
There was a problem hiding this comment.
I tend to push return statements to have an empty line before. This looks good though.
|
Looks great! Next time use a topic branch ;) |
Start exposing lists and package results via the API #132
|
Yeah, you know ... ever since my first github pull request (when I learned that additional commits to the branch become part of the pull request), I usually do that. I don't remember why I didn't in this case, other than the whole "I'm just fixing this one thing" mentality ;-) |
This is mostly so you can review and provide feedback ;-)