Code review comment for ~dashmage/charm-sysconfig:bug-2012581/inherit-existing-kernel-params

Revision history for this message
Ashley James (dashmage) wrote :

> I think a string with "=" assignments is not the key point in case 3. There are also "=" in case 1&2. The major different is double quote. Please mention that the subkey and value can be assigned by double quote.

There are "=" in the prev 2 cases but they do not occur in the values. Here in case 3,
'key1="subkey1=val1 subkey2=val2", key2=val3' -- the double quotes are just to make the assignment between key value pairs more clearer. The key point is that the subkey values themselves contain assignments that include "=". Even if the double quotes aren't present, the parser will correctly parse it into a dictionary.

> This case is weird. Don't we need the double quote?
'key1="subkey1=val1,val2 subkey2=val3"'

Here as well, the double quotes aren't really necessary but just to aid in identifying the key and value clearer. This case is to show how there's "=" assignments and the subkey is assigned multiple values separated by commas unlike the previous case which just contained a single value. The commas between val1 and val2 in "subkey1=val1,val2" is what this case is supposed to capture.

I've also added another unit test where the double quotes aren't present. I've addressed the other inline comments as well.

« Back to merge proposal