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 Azure table property being not sanitized. #1381

Merged
merged 1 commit into from Feb 5, 2016

Conversation

dVakulen
Copy link
Contributor

@dVakulen dVakulen commented Feb 5, 2016

Return values of pure method string.Replace weren't used.
Fixes #1385.

@dVakulen dVakulen changed the title Azure table property being not sanitized. Fixed Azure table property being not sanitized. Feb 5, 2016
key = key.Replace('/', '_'). // Forward slash
Replace('\\', '_'). // Backslash
Replace('#', '_'). // Pound sign
Replace('?', '_'); // Question mark
Copy link
Member

Choose a reason for hiding this comment

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

I know I'm nit picking, but could you put the dots before the method invocation, such as .Replace(

Copy link
Member

Choose a reason for hiding this comment

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

BTW, feel free to push back on the comment if it's going to require a lot of context switching or fighting git from you, this is a VERY minor comment that I don't feel absolutely must be addressed 👅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Already fought and won.

@jdom
Copy link
Member

jdom commented Feb 5, 2016

Thanks for the PR. The code looks good... now :)

jdom added a commit that referenced this pull request Feb 5, 2016
Fixed Azure table property being not sanitized.
@jdom jdom merged commit 99b1a88 into dotnet:master Feb 5, 2016
@jdom
Copy link
Member

jdom commented Feb 5, 2016

Thanks @dVakulen for the quick turnaround :)

@dVakulen dVakulen deleted the azure-table-prop-sanitizing branch February 5, 2016 19:17
@sergeybykov
Copy link
Contributor

👍

@gabikliot gabikliot added the bug label Feb 6, 2016
@gabikliot
Copy link
Contributor

Great catch by @centur !

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

Successfully merging this pull request may close these issues.

None yet

6 participants