[Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files

[Kai Köhne]

> -----Original Message-----
> From: Edward Welbourne <edward.welbourne at qt.io>
> Sent: Wednesday, February 15, 2023 10:45 AM
> To: Kai Köhne <Kai.Koehne at qt.io>
> Cc: Development at qt-project.org
> Subject: Re: Support for *Notes and UpstreamFiles fields in qt_attributions.json
> files
> 
> Kai Köhne (15 February 2023 08:50) replied:
> > did you intentionally sent this off-list?
> 
> oops - no, outlook's UI tricked me :-(
> And, in fact, it just did it again, although I'm sure I hit Reply All this time.
> Manually adding development to CC...
> 
> For the benefit of everyone else, my reply is below, after the bit of Kai's earlier
> mail, that I quoted:
> 
> >>> Anyhow, I wonder whether it wouldn't suffice to have _one_ comments
> >>> field, instead of a dedicated UpstreamFiles field, and *Notes fields
> >>> for every single entry. E.g.
> >>>
> >>> {
> >>>   "Comments": [
> >>>      "Upstream files were copied from:",
> >>>      "   src/dir1, src/dir2",
> >>>      "The license and copyright was derived from dist/LICENSE.txt"
> >>>   ]
> >>> }
> >>>
> >>> The benefit I see is that qtattributionsscanner (and any other JSON
> >>> tool that might be used by others) has only to care about one
> >>> additional field, not multiple ones.
> 
> Edward Welbourne (Tuesday, February 14, 2023 7:37 PM) had written:
> >> I see the case for it, but it comes at the cost of all the comments
> >> being in one place, not each comment next to the part of the content it
> relates to.
> >>
> >> If someone is updating one of many data in an attribution and the
> >> comments aren't close at hand, both the author of the change and the
> >> reviewers may fail to notice the detail that the comment mentions
> >> that makes it obvious they're doing something wrong.
> >>
> >> That's not a fatal argument: each attribution is fairly short, so
> >> putting the comments in the middle should make it easy to see them
> >> when looking at any of the lines they relate to.  None the less,
> >> comments and documentation belong close to the code they relate to ...
> 
> > Well, you can also achieve this by duplicating comment fields:
> >
> > {
> >   "Comment": "Upstream files are src/x.cpp, include/y.h",
> >   "Files": [ "x.cpp", "y_p.h"]
> >   "Comment": "Copyright info is from dist/COPYING",
> >   "Copyright": "Copyright (C) 2023 Joe Doe"
> > }
> 
> The problem with that is that I was given to understand that duplicated keys is
> actually malformed JSON - perhaps I misunderstood.  If that's legitimate JSON,
> then I'm fine with just one.

To my understanding it's valid JSON, at least from the syntax side. From https://www.ecma-international.org/publications-and-standards/standards/ecma-404/ :

  The JSON syntax does not impose any restrictions on
  the strings used as names, *does not require that name strings be unique*, and does not assign any
  significance to the ordering of name/value pairs. These are all semantic considerations that may be defined by
  JSON processors or in specifications defining specific uses of JSON for data interchange

And the JSON parsers I tested (Python, Qt) don't treat it as an error, either. There seems to be some online linters like https://jsonlint.com/ that complain about it, tough.

> > I just don't see the point of dedicated fields if the whole purpose is
> > to ignore them in the tooling.
> 
> Well, there's more tooling to consider - someone, at least, has run a JSON-
> validation checker on some qt_attributions.json files and submitted patches to
> fix issues reported there (like line-breaks instead of \n; and I think that's where
> I heard about duplicate keys being invalid) - and in any case there are
> developers who need to read it, who may benefit from the keys having names
> that makes clear which tooling-relevant keys they relate to.

Right. But this can also work the other way: If we ever come around to formalizing a json-schema, then having to list all the *Notes fields complicates things, too.

I'm still for the KISS principle and go for one Comment field. If we have a need for more 'local' fields in some cases, let's just allow duplicating these. We can always reconsider this in the future.

Kai