beer-song: Restore original property name

[rbasso]

Considering that this error would make automatic test generation
impossible, and that it is being fixed in a short time, we're not
bumping the MAJOR version.

Almost certainly nobody is already depending on the semantic
versioning of the test data.

What I wrote above make no sense at all. 😁

[kotp]

So how do we determine what "property" is supposed to hold. In some cases I see "integer" which could lean toward a technical thing in programming. In this case, property is "lyrics" which is definitely coined for the problem being solved.

I agree with this change, but there must be some way to document so that it is clear what that means, other than what is in the schema currently describing its character types.

[kotp]

To communicate a little bit further on why my choice of "verse" and "verses" I was describing the "property" based on what I was thinking at the time about "I expect to get a single verse, whatever that means as compared to down there, where I expect to get verses..." as an idea, but lyrics describes both forms of this, more generally.

[rbasso]

The safe choice for property is the name of the of the function implemented in the solution to be tested, in camelCase.

Some tests may not fit in this situation, then it needs to be decide for each case.

Edit: Most of the time I just take the name of the surrounding key.

[kotp]

So in this case, verse and verses?

[rbasso]

I agree that this doesn't really need to be changed, @kotp, but:

• verse and verses are too similar, so they could confuse people.
• The original group name was lyrics.
• @kytrinyx got confused about it.

So I opened this PR, but I really don't care about which one is the final name: lyrics or verses, so I'll let for you to decide to merge or close this PR. 👍

[kotp]

So it really could have gone either way? I think. I think the thing then that should be changed is the description then rather than the property?

[kotp]

Right, hehe, your response while I was expanding is exactly what I am gathering from this. What do you think @kytrinyx ?

[kotp]

I will mention that the influence here is the example solution, it uses verses, driven from the test. So it is bottom up driven, rather than top down driven, in this case.

[petertseng]

Would you explain for me...

Considering that this error would make automatic test generation impossible

Why is it made impossible? What will happen if I try?

What makes verses any more erroneous than lyrics?

I don't understand the answer to either question from looking at either canonical-data.json or the commit message, so maybe it will help people like me if the explanation of why verses makes generation impossible and lyrics is correct would go into the commit message.

Edit: Oh, is it because verses is too similar to verse? That's a good reason to pick lyrics, but it doesn't explain why it's impossible to use verses...

All I see from #661 (comment) is that the description says lyrics but the properties all say verses, but surely we aren't assigning any special meaning to the description field, becuase that means acronym/canonical-data.json makes no sense.

I understand the desire for greater consistency, but it doesn't explain to me why test generation is impossible.

[rbasso]

At first I imagined that everything was verse, @petertseng. Now I see that it was verse and verses.

I completely forgot about the comments, @kotp. Sorry for that!

I'll rewrite or kill this PR. What do you prefer?

[rbasso]

Sorry for all the confusion. I really didn't see it was verses instead of verse at first.
The PR is "fixed", so now it can be closed or merged. 👍

Edit: I need to stop writing PRs before having coffee in the morning. 😄

[kotp]

It is good this came up, I think... since it brings more attention to the need to explain the schema in a better way. Especially, "property" lately has been getting a lot of attention.

[rbasso]

I will mention that the influence here is the example solution, it uses verses, driven from the test. So it is bottom up driven, rather than top down driven, in this case.

This seems a good argument to immediately close this PR.

[kotp]

Closing as a non-issue, but a great source of information... hopefully to be converted into documentation.

[rbasso]

It is good this came up, I think... since it brings more attention to the need to explain the schema in a better way. Especially, "property" lately has been getting a lot of attention.

This confusion is mostly my fault. The only explanation about property is the README.md and it is fairly short and new.

[kotp]

Don't forget that the other documentation is the canonical.schema.json file as well. Anything that can be clarified in there directly is a good source. (The most direct reference to the subject at hand the better.)

[rbasso]

I will try to remember rewriting the comments there after we finish rewriting the canonical data files.

[kytrinyx]

I was confused about the inconsistency between the description and the property. This is partly because I was familiar with the history of the exercise, which has had both verses and lyrics where lyrics represents the external-facing expected API, and verses was considered a lower-level implementation detail.

If we're only going to have one, I don't care much which one it is, only that it's consistent :)

[kytrinyx]

To clarify—I was writing a generator for beer-song in Ruby, and needed to decide what to use to determine the name of the method. If that's property, that's all I need to know :-)