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

Removed Explicitly Setting Variable Type to Two Single Quotes #2425

Merged
merged 1 commit into from
May 1, 2019

Conversation

BeksOmega
Copy link
Collaborator

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

The details

Resolves

N/A

Proposed Changes

Removes explicitly setting the variable type to two single quotes.

Reason for Changes

Before the toXml/fromXml pr went in there was a check in domToFieldVariable_ to change an string containing two single quotes to an empty string, with a comment that maybe this wasn't necessary. I believe the only purpose of that chunk of code was to undo the thing that I am removing now (where the string containing two double quotes is created).

This all seems a bit weird though so I'm not sure if there was a secret purpose behind it. I did check the git blame and found the original PR, but I didn't see any information about why it was set up this way.

Test Coverage

I discovered this because field_variable's fromXml was throwing an errow when it was trying to deserialize the dynamic category XML, so I added tests for both the untyped and typed variable dynamic categories.

DynamicCategoryTests

I also manually tested creating variables through the flyout buttons, and that is working.
And I checked the test blocks' variable field block, and it is working.

Tested on:

  • Desktop Chrome

Additional Information

@rachel-fenichel
Copy link
Collaborator

This looks right. The actual reason for that code is completely lost to me, but as long as serialization continues to work (and it looks like it does) I agree with getting rid of this.

@rachel-fenichel rachel-fenichel merged commit 5e56b4a into google:develop May 1, 2019
@BeksOmega BeksOmega deleted the fixes/fromXml branch May 2, 2019 20:13
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.

None yet

2 participants