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

Fix for 20741. Allow older versions of tools to pass null args. #20799

Merged
merged 2 commits into from
May 5, 2020

Conversation

lajones
Copy link
Contributor

@lajones lajones commented May 1, 2020

Fixes #20741

Allow older versions of tools to pass null args. This should fix @ajcvickers comment. There may still be underlying issues with MigrationsModelDiffer.

@lajones lajones requested a review from bricelam May 1, 2020 03:21
@lajones lajones self-assigned this May 1, 2020
@lajones lajones changed the title Fix for 20741. Allow older versions of tools to pass null args. Partial fix for 20741. Allow older versions of tools to pass null args. May 1, 2020
@lajones lajones changed the title Partial fix for 20741. Allow older versions of tools to pass null args. Fix for 20741. Allow older versions of tools to pass null args. May 1, 2020
@ajcvickers ajcvickers requested a review from a team May 4, 2020 17:34
@lajones lajones merged commit d6c7be6 into master May 5, 2020
@lajones lajones deleted the 20200430_Issue20741_01 branch May 5, 2020 00:29

_reporter = reporter;
_assembly = assembly;
_projectDir = projectDir;
_rootNamespace = rootNamespace;
_language = language;
_args = args;
_args = args ?? Array.Empty<string>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Could also just do this once in OperationExecutor's constructor

Copy link
Contributor

Choose a reason for hiding this comment

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

(And leave the assert's in these classes)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bricelam This is already checked in. Do you think these changes are important enough for a new PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Meh. We're not likely to add any new operations classes anytime soon.

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.

remove-migration throws System.NullReferenceException
3 participants