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

Fixed #682 - AssemblySource throws ArgumentExceptions #687

Closed
wants to merge 3 commits into from
Closed

Fixed #682 - AssemblySource throws ArgumentExceptions #687

wants to merge 3 commits into from

Conversation

beachwalker
Copy link

... for duplicate keys. This catches the ArgumentExeption and continues instead of breaking. Also includes the DesignTime part, even though this never occurred there but keeps code the same style in both methods.

…eys. This catches the ArgumentExeption and continues instead of breaking. Also includes the DesignTime part, even though this never occurred there but keeps code the same style in both methods.
@beachwalker
Copy link
Author

Sorry, see my other issue #688 ... had to do by hand. How to run the checks locally? Works on my machine. ;-) How can I help to fix this? Don't know why net-core Bootstrapper has effects on the builds of Xamarin etc.

Thomas Stegemann added 2 commits April 15, 2020 17:34
Don't know why changes for net-core Bootstrapper have effects on the builds of Xamarin etc. This should make the code more or less equal to what was done before except for catching the Exception and doing Add() instead of AddRange(). This removes the check in the AssemblySource instance cache.
…problem of my commit, so reverted back to initial suggestion.

This reverts commit 56cf304.
@beachwalker
Copy link
Author

Sorry, see my other issue #688 ... had to do by hand. How to run the checks locally? Works on my machine. ;-) How can I help to fix this? Don't know why net-core Bootstrapper has effects on the builds of Xamarin etc.

The build error was not my fault... so I reverted back to the original submit.

@beachwalker
Copy link
Author

I would like to help you out but I'm not familar with the Xamarin platform. Do you need help there? Looking forward to see a new -alpha/-beta (-release?) version including my fix. ;-)

@@ -83,7 +83,19 @@ public void Initialize()
protected virtual void StartDesignTime()
{
AssemblySource.Instance.Clear();
AssemblySource.Instance.AddRange(SelectAssemblies());

foreach (var assembly in SelectAssemblies())
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the formatting is off here.

Copy link
Author

Choose a reason for hiding this comment

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

Looks good when looking at the file alone. I was not able to see in the file, only in the diff. I look for this tomorrow. As I wrote in the other issue I was not able to build/open using Visual Studio to check the solution. So pre-configured settings from you may not applied here.

Copy link
Author

Choose a reason for hiding this comment

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

@nigel-sampson
Copy link
Contributor

Actually wondering if we can push the check into AssemblySource itself and get the same protection across all platforms.

@beachwalker
Copy link
Author

Happy to see it there, but can not test the other platforms here. Maybe just go for TryAdd instead of Add?

@nigel-sampson nigel-sampson added this to the v4.0.0 milestone Jun 4, 2020
nigel-sampson pushed a commit that referenced this pull request Jun 4, 2020
@nigel-sampson
Copy link
Contributor

Pushed a fix to master.

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.

2 participants