TLDR: I've added args support to TryInvokeMember, and I think it is potentially very useful (I have two genuine use cases already, but it clearly supports many more), and so hopefully worth including in the project.
I've already written the code (indeed I'm using it live in a project), so since it's there and working already, I've already put it up as PR #279 for you to be able to have a look at it in your own time and see what you think.
Sorry for putting PRs and issues out of order here - as you realised in another thread, I'm fairly new to contributing to OSS - though not at all new to coding mode generally!!
As you've do doubt noticed (!), I've put up a small bunch of PRs which came about as I was working on my proposal for including additional stored procedure support in Massive. (Almost all of which you've kindly accepted already.)
While working on turning this stuff into separate PRs, I came up with the current proposal.
This was partly inspired by the brief discussion we already had, about your TestMaxOnFilteredSet #274 (I finally had to really get to grips with TryInvokeMember, to be able to make a useful issue report there!). Before that, I was already preparing a PR to propose adding columns support to the first of the two Single(...) methods in Massive.Shared.cs (the one which already allowed where and args as params), noting that the other explicit variant of Single(...) already allowed columns, that it was a useful feature (at least to me), and that it was trivial to add there to.
But since you'd already just added where support to TryInvokeMember, and since my mind was already on how that method worked, the penny dropped that adding args support to TryInvokeMember would actually allow me to completely delete the version of Single(...) which I was modifying (since TryInvokeMember would now support it), and would also support args in places like your MaxOnFilteredSet test, where they might we be expected to work (by someone learning Massive), but previously didn't.
So I made the change, checked it didn't break any existing tests, checked it worked with my exisiting live project using Massive, and added some new tests. The result is PR #279.
mikebeaton
commented
7 years ago
Dear Frans,
Hi, and another one.
TLDR: I've added
argssupport toTryInvokeMember, and I think it is potentially very useful (I have two genuine use cases already, but it clearly supports many more), and so hopefully worth including in the project.I've already written the code (indeed I'm using it live in a project), so since it's there and working already, I've already put it up as PR #279 for you to be able to have a look at it in your own time and see what you think.
Sorry for putting PRs and issues out of order here - as you realised in another thread, I'm fairly new to contributing to OSS - though not at all new to coding mode generally!!
As you've do doubt noticed (!), I've put up a small bunch of PRs which came about as I was working on my proposal for including additional stored procedure support in Massive. (Almost all of which you've kindly accepted already.)
While working on turning this stuff into separate PRs, I came up with the current proposal.
This was partly inspired by the brief discussion we already had, about your
TestMaxOnFilteredSet#274 (I finally had to really get to grips withTryInvokeMember, to be able to make a useful issue report there!). Before that, I was already preparing a PR to propose addingcolumnssupport to the first of the twoSingle(...)methods inMassive.Shared.cs(the one which already allowedwhereandargsas params), noting that the other explicit variant ofSingle(...)already allowed columns, that it was a useful feature (at least to me), and that it was trivial to add there to.But since you'd already just added
wheresupport toTryInvokeMember, and since my mind was already on how that method worked, the penny dropped that addingargssupport toTryInvokeMemberwould actually allow me to completely delete the version ofSingle(...)which I was modifying (sinceTryInvokeMemberwould now support it), and would also supportargsin places like yourMaxOnFilteredSettest, where they might we be expected to work (by someone learning Massive), but previously didn't.So I made the change, checked it didn't break any existing tests, checked it worked with my exisiting live project using Massive, and added some new tests. The result is PR #279.