Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Castle.Core dependency to 4.0.0 #235

Merged
merged 6 commits into from
May 18, 2017

Conversation

alinapopa
Copy link

Changes:

  • Remove System.Security.AllowPartiallyTrustedCallers attribute because Castle.Core does not have it
  • Update references to Castle.Core, Castle.Core-log4net and Castle.Core-NLog to latest
  • Fix issues caused by breaking changes in Castle.Core 4.0.0

{
return (TAttribute[])Attribute.GetCustomAttributes(item, typeof(TAttribute), true);
return (TAttribute[])Attribute.GetCustomAttributes(item, typeof(TAttribute), inherit);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this to avoid ambiguity because Castle.Core also has a GetAttributes() extension method, and it passes false for inherit

@@ -307,7 +308,7 @@ private static AssemblyName GetAssemblyName(string filePath)
private static TBase Instantiate<TBase>(Type subtypeofTBase, object[] ctorArgs)
{
ctorArgs = ctorArgs ?? new object[0];
var types = ctorArgs.ConvertAll(a => a == null ? typeof(object) : a.GetType());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ConvertAll() extension method from Castle.Core (CollectionExtensions.cs) no longer exists.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added EnumerableExtensions.cs

using System;
using System.Collections.Generic;

static class EnumerableExtensions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explicit mark this internal.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed


static class EnumerableExtensions
{
public static Y[] ConvertAll<X, Y>(this X[] items, Func<X, Y> converter)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you change X/Y to T/TOutput, whatever the BCL convention is for this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to T/TResult

results.Add(y);
}

return results.ToArray();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea how often this method can get called? Would be much nicer on the GC to allocate an array and populate it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

public static void ForEach<X>(this IEnumerable<X> items, Action<X> action)
{
foreach (var item in items)
action(item);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curly braces.

return results.ToArray();
}

public static void ForEach<X>(this IEnumerable<X> items, Action<X> action)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change X to T.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -0,0 +1,42 @@
// Copyright 2004-2011 Castle Project - http://www.castleproject.org/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2004-2017

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@@ -267,6 +267,9 @@ public void CanResolveClientAssociatedWithChannelUsingSuppliedModel()
}

[Test]
#if DEBUG
[Ignore("This test fails in Debug / relies on GC.Collect to clean weak reference")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is somewhat worrying that GC.Collect no longer does what it says on the tin, we've never had a problem with this in the past. Any idea what has changed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, in Debug mode the JIT will choose to keep the local variable alive until the end of the method, so that it can be inspected in the debugger. This is not new, it's likely this test was failing before.

@jonorossi jonorossi merged commit 717be97 into castleproject:refactor-build May 18, 2017
@jonnii
Copy link

jonnii commented May 18, 2017

🎉 🎉 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants