cps-org/cps

Keys should be all lower case (or at least the case should be required)

dcbaker opened this issue ยท 11 comments

The spec currently allows keys to be any case, but recommends mixed case. This is a bad idea. For one, it's much harder to write a schema for, and therefore harder to both validate and autocomplete. It also means that when a consumer reads the CPS, if their json implementation doesn't support case-insensitive lookup (I have yet to see one that does), they have to recursively walk the keys, lower case them, and hope there's no collisions.

#2 is the sort of change I don't want to unilaterally make, even though I'm inclined to agree with it. "A community model of changes" doesn't just mean "not blocked by an unresponsive maintainer", the community needs to exist and be active in discussions. I'm all for improvement in that respect!

FWIW, I felt your pain when I did the CMake demonstration implementation... but even if CPS does become case-strict, I don't know that I'll rip it out. BLIWYA...

My day job is working on Mesa. OpenGL was no shortage of pain because everyone developed games against NVIDIA, and we essentially had to just do whatever NVIDIA did, regardless of what the spec said. Often the spec was changed because we discovered that no one actually implemented it, and games in the wild depended on what NVIDIA did, and thus what everyone had actually implemented. CMake is likely to be the reference implementation, and people will rely on whatever behavior CMake has, and as a result I as an implementer will have to follow CMake's behavior.

My experience with OpenGL (and later with Vulkan where people went out of their way to not get into this situation) make me really nervous about handy waviness in a spec. We should be clear what is and what isn't allowed.

In this case I think CPS is still at a point that we could make this sort of change without major breakage. The longer we wait the harder it becomes to do so.

I like this issue. We should make a specific call on this question. Next step would be someone bringing the issue up in a future Ecosystem Evolution meeting. This is an open group, so more contributors are welcome.

Feel free to reach out to me if you have any questions or concerns.

I think it's more appropriate to discuss specific issues with CPS in the CPS issue tracker on github or in an MR on github, than to move them to a mailing list. When the issue is closed, either by a MR or because a decision to leave things in place are made, the history of that discussion, and the rational for the change are in a single place. Because it is common practice to use the Closes: #2 keyword syntax, a link is made in the git history to the MR, and thus to the issue. That is all preserved, and if anyone in the future is curious as to why a specific decision was made they can see that easily.

I don't think we're proposing moving this to a mailing list, but bringing it up in an actual meeting (IIRC I've tried to do that already but we didn't have time). I think that's more likely to gain traction, if only by virtue of having a semi-captive audience. However, we would certainly record any decisions (and rationales, as appropriate) here. That said, I think the rationales for both options are fairly clear; the question is rather which the community weights more heavily.

I have concerns that making decisions in scheduled meetings reduces the ability of people to comment on proposals since it locks in a time and date that may or may not be feasible for everyone who wants to be involved. However, I'm willing to give it a try and see what that would look like for a project that is being done at least in part by volunteers (like me). It might be worthwhile to discuss in such a meeting how we foresee the interactions between "we discussed this in the EE meeting on 25/1/1" and "other parties commenting on github" going, since I think it's likely if CPS does gain traction that scheduling meetings may become more difficult as we add contributors in conflicting time zones.

Those are fair concerns. Feel free to reach out if anything seems dysfunctional.

The monthly meeting is mostly about creating a space for bootstrapping a community around these concerns and ideas. Live discussion seems to be a good fit for driving convergent thinking, not that there aren't other ways to do that. It's also a good way to draw attention to at least those available to participate. Scheduling can be a concern, but so far we have had good luck at least including participants from Western Europe and Western America.

We also have an #ecosystem_evolution channel in https://cpplang.slack.com if anyone likes that better, though that sort of discussion has other tradeoffs. The Google group also exists to provide longer form async discussion accessible to anyone with an email address. Participants in the monthly meetings hang out in those channels (and reddit and HN and ISO meetings and here and so on). Getting thoughts and research shared anywhere is a good place to start.

So far, nuts and bolts, the project owners of the CPS repo are merging PRs and therefore making the ultimate choice about the CPS spec. But since a project goal is to provide interop across tools and ecosystems, concerns from all sorts of perspectives are taken seriously.

I have concerns that making decisions in scheduled meetings reduces the ability of people to comment on proposals

People are, of course, welcome to comment here, and I have no objection to creating issues and/or using the mailing list to discuss important decisions... but note how many people have participated in this issue. I see the goal of using the meetings as a way of twisting peoples' arms to contribute when other mechanisms are failing to produce discussion.

The the cons and pros of using case insensitive key are:
Pros:

  • A user could write Configuration instead of configuration (And iiuc it is not expected that users write cps files manually)

Cons:

  • Hard to parse. Json keys are not case insensitive. You would have to list all keys and search in the list while ignoring case.
  • What about that? Valid json and valid keys:
{
  "configuration": {},
  "Configuration": {}
}
  • You can not write a json schema that is case insensitive. This means:
    • No auto-completion in editors
    • Unable to use json schema validators to validate cps files

Given these pros and cons the decision seems very simply to me, but maybe I missed something?

Okay, between my own inclination and several votes in favor, a future version of CPS will remove case-insensitivity on key names. Folks also seem to strongly prefer snake_case, so...

Needless to say, this will be a significant revision, so probably won't happen immediately.

Ok so should I adapt #25 and change for example the compile-features key to compile_features?

so should I adapt #25 and change for example the compile-features key to compile_features?

@autoantwort, yes, please-and-thank-you! Also, sorry I haven't had a chance to review that; I'm generally in favor of the idea, but want to verify that the schema is correct. (Help would be appreciated!) Also, given the change, we might want to hold off merging until the schema specification is updated per this issue.