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

Issue #1665: added null validation #1666

Merged
merged 3 commits into from
Apr 6, 2022
Merged

Conversation

brunomartinspro
Copy link
Contributor

@brunomartinspro brunomartinspro commented Apr 4, 2022

Added a validation for additionalImportDirs before making a split.

Fixes #1665

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 4, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: brunomartinspro / name: Bruno Martins (17e5c29)

@brunomartinspro
Copy link
Contributor Author

brunomartinspro commented Apr 4, 2022

Hi @JamesNK !

when you have the time can you take a look?

thank you in advance

@JamesNK
Copy link
Member

JamesNK commented Apr 5, 2022

Change looks fine, but it would be good to verify with a test to make sure it doesn't regress. Are you comfortable with adding a unit test? There is an existing test project with the command-line tool.

@brunomartinspro
Copy link
Contributor Author

brunomartinspro commented Apr 5, 2022

@JamesNK sure! I will add a unit test for this case.

@brunomartinspro
Copy link
Contributor Author

@JamesNK Added the Unit Test "AddProtobufReference_Without_AdditionalImportDirs" based on "AddProtobufReference_AdditionalImportDirs" but with the NULL case.

test/dotnet-grpc.Tests/CommandBaseTests.cs Outdated Show resolved Hide resolved
Co-authored-by: James Newton-King <james@newtonking.com>
@JamesNK JamesNK merged commit 737f712 into grpc:master Apr 6, 2022
@JamesNK
Copy link
Member

JamesNK commented Apr 6, 2022

Thanks!

@brunomartinspro
Copy link
Contributor Author

@JamesNK thank you as well! 🚀

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.

Exception Thrown on string.Split
2 participants