Defines should be hash instead of array

[KalleDK]

Hi, I've poked around in the sourcecode to see how easy it would be to change defines from an array to a hash.

Reason to do this is:

• Yaml has a <<: merge option with hashes, instead og the array in arrays it is now (if you try to make a common list)
• Defines should be unique, you can easily (by accident) merge to arrays, where the same define has to values. If we changed to hash, it would be easy to update a define.

:defines:
  :common: &common_defines
    :DEBUG:
    :HZ: 20
    :COMP: "( A > B )"
    :FILE: '\"hed.h\"'
  :release:
    <<: *common_defines

-DDEBUG -DHZ=20 -DCOMP="( A > B)" -DFILE=\"hed.h\"

I would love to do the work, but I would also love to have some feedback, before i commit to much time to it.

[austinglaser]

I think this sounds like the right decision. However, there'll have to be some thought to backwards compatibility (we don't want to suddenly break everyone's config file)

[mvandervoord]

I like this idea a lot. You will want to fall back to support an array if already supplied. Also, you'll want to accept keys that are symbols OR strings (converting both to strings, most likely).

I'll definitely merge this feature when you put it together well. :)

[KalleDK]

The backward combability is gonna be a pain, any reason not to include a version in the project.yml and thereby make it easier to make a breaking change like this ? I think I would be able to do it, with the backward, but it should only live for a "short" time as it would make the code somewhat harder to read.

The symbol and string, I can follow the logic, but I would say there should be a restriction to either one or another... So you won't be able to do something like this

irb(main):155:0> defines
=> {"t"=>5, :t=>6}

[austinglaser]

The way I've seen this dealt with elsewhere (where, for instance, both a string or an array of strings is legal) is that somewhere early in the config processing one type is converted to the other. When defines are given as an array, we could split each array element on the equal sign, use the first element as the hash key, and the second as the value. Then all the internal code can just handle a hash of defines. On May 23, 2016 01:36, "Kalle Møller" notifications@github.com wrote:

The backward combability is gonna be a pain, any reason not to include a version in the project.yml and thereby make it easier to make a breaking change like this ? I think I would be able to do it, with the backward, but it should only live for a "short" time as it would make the code somewhat harder to read.

The symbol and string, I can follow the logic, but I would say there should be a restriction to either one or another... So you won't be able to do something like this

irb(main):155:0> defines => {"t"=>5, :t=>6}

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/ThrowTheSwitch/Ceedling/issues/80#issuecomment-220919349

[mvandervoord]

I agree with Austin. The trick to these kinds of changes is to convert it to the type you want as EARLY in the process as you can... soon after loading the yaml, usually. This way we maintain backward compatibility, plus we make the entire thing simpler to use (It "Just Works" no matter which way you specify it).

[KalleDK]

Right so you would agree if I early on convert the array to hash, and the keys, which are strings into symbols.. And convert the rest of the program to use this ?

[mvandervoord]

Yes, I think it would be awesome.

(Though I might consider converting from symbols to strings since they need to be strings in the long run anyway and they're never really used for comparisons)