@dotnet-policy-service agree

@aivascu The pipeline is failing due to previous .NET versions, and I've noticed recent PRs adding .NET 5 runtime like on #1443 being merged to the release branch.

May this PR be based on release/5.0.0 instead of the master branch? Couldn't find info about it on the code docs.

I was able to build, test, and pack it locally but the cmd script fails.

@luccanog the errors thrown by the pipeline are related to the SDK building the code. Looks like AppVeyor have updated their VS 2019 image. I'll add the errors to ignore list.

Looks like they outright removed .net core 1 and 2, and I've just finished re-balancing all versions on the release branch...
@luccanog could you rebase your PR on the release branch and target that instead of the master?

@aivascu The rebase is done.
I've noticed the Readme file was added up because it was modified on the master branch, please let me know if I should undo those changes or if I should keep them.

@luccanog yes please undo the readme. I'll synchronize them in a separate PR.

@aivascu I came back with the old content, but for some reason git hub still thinks there is a difference between files. It might be irrelevant to the process

@luccanog I've rebased and squashed your commits on the release branch. I hope you don't mind.

I've had a brief look at your implementation, but I'm not done reviewing it for now.
I'm not convinced that piggybacking on the factory function overload is the way to go. As a workaround it might work fine, but there must be a more efficient way to compose the nodes than that.

Will let you know if I come up with anything.

@aivascu No problem at all, feel free to do any other rebase if needed.

About he piggybacking, do you mean the changes at the NodeComposer class? There I re-used an existent overload since the method has got what we need, but of course we have some other ways. As I see, we could call the WithCommand method directly at our new With method, and still use the same public BindingCommand signature.

In this way our new With method is going to be pretty similar to the existent one, but with a conversion from ISpecimenBuilder to a Func<TInput, TProperty>.

Please let me know when you got any thoughts.

@luccanog yes, I meant that you reused the builder syntax, to create a factory that's using the specimen builder. 😅
Sorry I should be more explicit.

Using the BindingCommand sounds better however I propose that we add a new overload for the public constructor.

public BindingCommand(Expression<Func<T, TProperty>> propertyPicker, ISpecimenBuilder builder)
{
    if (propertyPicker is null) throw new ArgumentNullException(nameof(propertyPicker));
    if (builder is null) throw new ArgumentNullException(nameof(builder));

    this.Member = propertyPicker.GetWritableMember().Member;
    this.ValueCreator = c => (TProperty)builder.Create(typeof(TProperty), c);
}

This would allow to pass the context directly into the builder and avoid going through several layers of abstractions.
I tried it locally and it seems to be working fine.

Let me know if you'll try these changes, and cover them with tests.

I just implemented the new BindindCommand conStructor and added more tests as requested. Seems to work properly.
Now we don't have to re-use the With method overload, and we can directly construct the BindingCommand with a ISpecimenBuilder

@luccanog after some consideration, I decided I don't want yet to couple BindingCommand to ISpecimenBuilder, so I reverted the changes and used the existing constructor.

@aivascu alright. Thank you!