These are chat archives for chocolatey/chocolatey-oneget

3rd
Mar 2015
Joel Bennett
@Jaykul
Mar 03 2015 01:16
@ferventcoder you around?
Rob Reynolds
@ferventcoder
Mar 03 2015 02:21
Nope - but I can catch up later.
Alternatively we can schedule some pairing for design/changes if that's what you are looking for @Jaykul
Joel Bennett
@Jaykul
Mar 03 2015 02:29
Yeah, maybe ...
I want to get this done
But I don't feel like we have an agreement on the new interface/api
And there's no point doing it if you aren't going to take the pull request ;-)
Rob Reynolds
@ferventcoder
Mar 03 2015 03:40
I wouldn't say that we don't have an agreement. If you are referring to the comments - that's pretty standard.
The commit messages will def need fixed up. They don't meet the guidelines. Alos standard expectation considering the contributing doc spells its out
s/Alos/Also
We def need something that returns values from choco.
And the current api is much more fire and forget which is not going to work for all cases (most cases?)
It was literally put together quickly as an early early preview
Joel Bennett
@Jaykul
Mar 03 2015 03:46
I was always planning on squashing the commits down, so changing the commit messages is no big deal, I wanted to keep them separate because I felt it was easier to follow what was changed and why
Rob Reynolds
@ferventcoder
Mar 03 2015 03:47
I agree in keeping them separate when meaningful.
Joel Bennett
@Jaykul
Mar 03 2015 03:47
But I still like the idea of having ICommand and IListCommand separately, so I can alter one command at a time to implement the list method
(bearing in mind that IListCommand derives from ICommand, so it's still a command, with just list as extra
Rob Reynolds
@ferventcoder
Mar 03 2015 03:48
Yeah and maybe thats best for now. Thats what I meant by starting to understand.
Joel Bennett
@Jaykul
Mar 03 2015 03:48
The hardest part is the last outermost layer
Rob Reynolds
@ferventcoder
Mar 03 2015 03:49
I got that. For me it was trying to understand what would call ChocoSourceCommand.list
Joel Bennett
@Jaykul
Mar 03 2015 03:49
Right
Rob Reynolds
@ferventcoder
Mar 03 2015 03:49
The listrunner I guess is making that call?
Joel Bennett
@Jaykul
Mar 03 2015 03:49
Right
And that's (only) being called by the Sources() function
Rob Reynolds
@ferventcoder
Mar 03 2015 03:50
Turtles all the way down
:)
Joel Bennett
@Jaykul
Mar 03 2015 03:50
I think it could also be called by a Packages() function ... at which point, we could probably call the function List<T>()
instead of Sources and Packages
Rob Reynolds
@ferventcoder
Mar 03 2015 03:50
Yes
Joel Bennett
@Jaykul
Mar 03 2015 03:50
I was looking at that just now to see if that would work
I would rather if there was only one runner
because it's so copy/paste
but perhaps it would be easier to refactor common parts
Rob Reynolds
@ferventcoder
Mar 03 2015 03:51
Ya
Joel Bennett
@Jaykul
Mar 03 2015 03:52
So instead of GenericRunner and ListRunner, we just have run and list on GenericRunner
if that all makes sense to you
Then I'll start refactoring and slicing and dicing and fix or make a new pull request ;)
I guess I can push --force to totally change the existing pull request, but that's just ... weird
Rob Reynolds
@ferventcoder
Mar 03 2015 03:54
Thats normal :)
Joel Bennett
@Jaykul
Mar 03 2015 03:54
Heh, ok
Rob Reynolds
@ferventcoder
Mar 03 2015 03:54
When your full time job centers around github
Joel Bennett
@Jaykul
Mar 03 2015 03:55
Yeah, everything yells at me when I push --force, but it makes sense for pull request branches.
Rob Reynolds
@ferventcoder
Mar 03 2015 03:56
And only then :) - keeps the conversation and history right there
Joel Bennett
@Jaykul
Mar 03 2015 03:56
yeah
I made you one new one though ;-) chocolatey/choco#140
Joel Bennett
@Jaykul
Mar 03 2015 04:18
Right now, the runner throws an exception if it can't find a command and the CommandName is set, but does NOT throw an exception if the CommandName is not set. Is that for a reason?
Rob Reynolds
@ferventcoder
Mar 03 2015 04:34
Pass through
Joel Bennett
@Jaykul
Mar 03 2015 06:32
Hey, what's the deal with stable and master -- you want this pull request on top of stable or master?
Gary Ewan Park
@gep13
Mar 03 2015 07:17
Joel Bennett
@Jaykul
Mar 03 2015 07:40
Yeah, or makes me more confused
The changes from @mwrock didn't go on master
Gary Ewan Park
@gep13
Mar 03 2015 07:42
right, the changes that Matt is making is going into the next vNext release, i.e. 0.9.9, so he needed to target stable, as they will go out immediately, once the RC round is completed
as he originally targetted master with his PR
Joel Bennett
@Jaykul
Mar 03 2015 07:46
can you merge those to master?
Gary Ewan Park
@gep13
Mar 03 2015 07:47
yes, I believe so. That is some git fu that I don't know how to do yet, still very much learning, but i think @ferventcoder has the power to do this
Joel Bennett
@Jaykul
Mar 03 2015 07:52
hehe
Well, I'm going to just blame @ferventcoder
He asked me to pick up the change from @mwrock, so I'm working on stable because it's ahead of master :-p
Joel Bennett
@Jaykul
Mar 03 2015 08:17
Well, that's 2 out of three CI builds finished and green, I'm going to bed instead of waiting for the last one :sparkles:
Gary Ewan Park
@gep13
Mar 03 2015 08:20
coolio! We know where you live if that last one fails :-P
Joel Bennett
@Jaykul
Mar 03 2015 08:26
Ugh. The command "nuget restore src/chocolatey.sln" failed and exited with 1 during ....
travis might hate you
or me
I'm not sure which
g'night
Gary Ewan Park
@gep13
Mar 03 2015 08:27
ha ha :-) yeah, let's not worry about that one just now
have a good one!
Rob Reynolds
@ferventcoder
Mar 03 2015 13:07
@Jaykul travis ci has been hating on nuget restore lately
Joel Bennett
@Jaykul
Mar 03 2015 17:18
So, I gotta reverse your question on you -- you asked why I wrote that one function as a yield result -- but why is your NugetService.list_run NOT written as yield result? Your NugetList.GetPackages is taking an IQueryable which returns results as they come over the wire, and then calling .ToList() to force you to wait until you get all the answers....
why are you calling .ToList instead of just outputting the data as it comes over the wire.
Rob Reynolds
@ferventcoder
Mar 03 2015 18:50
probably because I wasn’t thinking ;)
actually there may have been a specific reason there
Joel Bennett
@Jaykul
Mar 03 2015 18:59
It means I don't get any output until the full list is retrieved
Rob Reynolds
@ferventcoder
Mar 03 2015 18:59
I’m not sure that is true
sorry
I’m not sure I see that as true when running choco list commands
Joel Bennett
@Jaykul
Mar 03 2015 19:00
?
Of course it's true
NugetCommon.GetRemoteRepository(configuration, nugetLogger).ToList()
Nothing can come back until they have all come back
try: choco list sub
Rob Reynolds
@ferventcoder
Mar 03 2015 19:02
yeah, will need to fix
I made that change somewhere along the way
chocolatey/choco@226e998
Joel Bennett
@Jaykul
Mar 03 2015 19:03
By the way, I meant to ask yesterday -- are there tests which test the actual output of the commands like source list?
Rob Reynolds
@ferventcoder
Mar 03 2015 19:03
Found it
Maybe? The test coverage isn’t where I want it to be.
The integration tests however cover a ton of closer to black box scenarios
Joel Bennett
@Jaykul
Mar 03 2015 19:04
yeah, I just wanted to see a couple, because ... well, I added a list to ensure that source_list is called
But obviously I need one with a mock list of sources to ensure the list is returned
Just, you know, to make sure nobody breaks my change in the future ;-)
Rob Reynolds
@ferventcoder
Mar 03 2015 19:06
chocolatey/choco#143
Joel Bennett
@Jaykul
Mar 03 2015 19:08
heh
But ... why!?
Joel Bennett
@Jaykul
Mar 03 2015 19:34
The awesome thing is you call ToList on that at least three times :fearful:
There must be some history there
Rob Reynolds
@ferventcoder
Mar 03 2015 19:53
It was like DIAF for some reason