Amazing, thanks Miguel, comments inline.

On 2 Nov 2014 00:52, "Miguel de Icaza" <mig...@xamarin.com> wrote:
>>
>>
>> PR1349: https://github.com/mono/mono/pull/1349
>> This is the machine key work, and needs a small tweak before it can be
merged that I will do this week.
>
>
> I believe the TODO can be removed.   Can you do that?  See comments on
pull request.

I don't believe the TODO can be removed as the Encrypt Decrypt methods
don't allow you use custom decryptors (from what I could tell) it's 4.5
specific functionality and controlled via the Web.config.

Do you want me to expand on the TODO to make it clearer?
>
>>
>> PR1363: https://github.com/mono/mono/pull/1363
>> Another of mine with the MembershipPasswordAttribute
>
>
> Do you mind resending this?  It can no longer be auto-merged from the UI.

Before I send this, I need to change the build order in the mcs/class
makefile, and wanted to run that past you first as it strikes me as
something via fragile.  I sent a message to the list a few days about it,
could you respond on that and I'll resend?
>
>>
>> PR1365: https://github.com/mono/mono/pull/1365
>> This is Kornel Pal's around the HttpTaskAsyncHandler, and Miguel has
said he'll take a look at it.
>
>
> Manually aded
>
>>
>> PR1370: https://github.com/mono/mono/pull/1370
>> Small one implementing a default of the ReadEntityBodyMode
>
>
> Got this one by hand.
>
>>
>> PR1371: https://github.com/mono/mono/pull/1371
>> Another small one, implementing the ClientDisconnectedToken
>
>
> And this one automatically.
>
>>
>> PR1372: https://github.com/mono/mono/pull/1372
>> A final small one around the GetBuffer* methods in the httprequest.
>
>
> I do not like this one.  Is there a reason we can not implement the
required functionality instead?
It's not actually required as the default on ReadEntityBodyMode means that
all clients of the code should go directly to input stream.  The code here
is enough to ensure that calling code works.

Looking at various posts about how to used the GetBufferlessInputStream,
it looks like it needs Async reads as well as Synchronous, so it's a little
more involved.

The only thing that we could potentially do is change the exception to one
you get when you try to read it buffered when classic is set.
>
> Miguel
>>
>> There is 1 final small piece that either myself of Chris Carroll will
get done this week which is around the AppendTrailing slash and
lowercaseUrls properties in RouteBase class.  We just need to put the
implementation together.
>>
>> Anyway, after applying all of these, my large WebAPI solution not only
compiles, but it also runs!
>>
>> If you want to checkout what it looks like with all the patches applied,
that would be great, I'd love to have some more information on whether it
does work.  I'm sure there will still be bugs, but if it works mostly, then
bug fixing is easy (famous last words).
>>
>> https://github.com/martinjt/mono/tree/mvc_allfixes
>>
>> Thanks for everyone's help.
>>
>> Martin
>>
>> On 20 October 2014 20:42, Martin Thwaites <monofo...@my2cents.co.uk>
wrote:
>>>
>>> Hi Miguel,
>>>
>>> The code that I'm referring to here is that of the aspnetwebstack on
codeplex.  That is to say that they are not something where you can remove
the code and recompile (unless there as a specific mono implementation
which is not ideal).  The goal is to have the compiled dlls that are
available on nuget work, without tweaking to a person's application.
>>>
>>> I'll have a look and see if I can see where it would be used, but still
as you've said on one of my pulls, a half done implementation is better
than none.
>>>
>>> Having the application throw a missing method exception should not be
the recommended approach when we can add the property and default it to
false.
>>>
>>> Thanks, and please don't think that things won't getting better with my
reviews.  I'm learning what you want so I can review better and help reduce
the burden on you and your staff.
>>>
>>> Martin
>>>
>>> On 20 Oct 2014 20:04, "Miguel de Icaza" <mig...@xamarin.com> wrote:
>>>>>
>>>>> As for the properties, although they should do something to the
generated urls, simply adding them should surely be a valid pull?  the
issue at the moment is that without them, you get an exception even if it
should be false.  I actually think that these are used by other classes
when generating urls, not the route collection itself, but I don't know for
sure.  Considering that adding them is very low risk, can we not just
accept the pull and ask for further work.
>>>>
>>>> Nope, all they do is allow some code to be compiled, and then get the
wrong result.
>>>>
>>>> You might as well remove the dependency of those properties, and see
what else breaks on whatever piece of code you are trying to build.
>>>>
>>>> Miguel
>>
>>
>>
>> _______________________________________________
>> Mono-devel-list mailing list
>> Mono-devel-list@lists.ximian.com
>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>
>
_______________________________________________
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list

Reply via email to