martintmk
martintmk
Thanks Mark, I am all for simplicity :). If your approach is still able to internally reconstruct and reuse the hash, then it's a better, simpler approach. I will give...
@mgravell , I did take a look at the implementation and I think the approach you proposed should work as we will be hitting this code-path (the hashed value is...
> Re key prefix; the existing implementation also didn't use any privileged APIs - it just expanded them as part of ExtractParameters - so IMO we should just keep doing...
@mgravell After checking this with the team we are still kinda worried about the extra CPU cycles consumed by `ResultProcessor.ScriptLoadProcessor.IsSHA1(script)`. Do you think we can still re-use the hash in...
> Re key prefix; the existing implementation also didn't use any privileged APIs - it just expanded them as part of `ExtractParameters` - so IMO we should just keep doing...
> > If you're asking about hiding the hash property: yes, we can safely IMO hide that in every way - [Browsable], [EditorBrowsable] and [Obsolete] (with the latter as a...
> It's a great question, but I think I'd rather defer on that for now Sounds good, pushed the changes and reverted any API additions. Let me know if the...
> Can you please add the `.Length == 40` check as per the comments above? thanks Done.
@mgravell Pushed the changes. Anything left to address? Unfortunately, some random tests failed on Ubuntu should I just retry?
@NickCraver , @mgravell any chance of taking a quick look at the proposed PR? So that I can continue work on it. Thanks :)