Switch to YAML & move appindicator to shared-modules

[ZVNexus]

No description provided.

[ZVNexus]

@TingPing i bring more changes

[ZVNexus]

Blocked on flathub/shared-modules#319

[flathubbot]

Started test build 150852

[flathubbot]

Build 150852 failed

[Erick555]

moving to yaml would break git blame in case someone wants to see historical context of some part of manifest.

it also makes reviewing other changes harder

[ZVNexus]

moving to yaml would break git blame in case someone wants to see historical context of some part of manifest.

What I typically do here is just git follow -- file.json and checkout to the file before it was modified, then you can blame it.

Small price to pay for YAML, JSON really really sucks. Kept forgetting commas, though you could say skill issue.

[Erick555]

but 99% people looking at it will do it in github, not cli.

yaml is so friendly that it doesn't warn when you break indentation and introduce subtle bugs

[ZVNexus]

but 99% people looking at it will do it in github, not cli.

In that case, you can go to the .yaml file in your browser, find the first commit in it's history, browse files, view history again, and browse files before that commit and then you can blame the JSON

Scales with number of commits obviously though

[Erick555]

I know but then something taking 1 minute takes 10 min and you may give up.

[Erick555]

at least you may split converting to separate PR

[ZVNexus]

at least you may split converting to separate PR

Sure, I'll split this up into separate PRs

[Erick555]

I was thinking about splitting 6cd1d1a and leave the rest combined.

every update results in all users re-downloading whole snap from canonical servers so tiny cosmetic issues aren't worth separate PR.

[flathubbot]

Started test build 150872

[flathubbot]

Build 150872 failed

[ZVNexus]

I was thinking about splitting 6cd1d1a and leave the rest combined.

The idea was to have the YAML commit be dependent on the appindicator inclusion in shared-modules.

every update results in all users re-downloading whole snap from canonical servers so tiny cosmetic issues aren't worth separate PR.

Eh, as long as they're merged relatively close together it should still make it all in the same update for the end user, Flatpaks don't get updated on a machine the moment one is available.

Also the snap is only 180MB, it really doesn't matter.

[flathubbot]

Started test build 151004

[flathubbot]

Build 151004 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/134089/com.spotify.Client.flatpakref

[ZVNexus]

I was thinking about splitting 6cd1d1a and leave the rest combined.

Done: #305

[joshua-stone]

moving to yaml would break git blame in case someone wants to see historical context of some part of manifest.

it also makes reviewing other changes harder

I agree that this change would break continuity when viewing changes over the years of history that's been established. JSON is also still much better supported in older flatpak-builder installs and is still the default format for new projects created in Gnome Builder. Those points alone make JSON still compelling.

That being said, I will say that in general, I find YAML is simply an easier format for day-to-day maintenance:

A) YAML is much more concise, even when a lots of line breaks are used. For large files, you're looking at easily saving 25%-30% in lines:

$ python3 -c 'import requests, yaml; req = requests.get("https://raw.githubusercontent.com/flathub/com.spotify.Client/refs/heads/master/com.spotify.Client.json"); print(f"JSON: {req.text.count("\n")}, YAML: {yaml.safe_dump(req.json()).count("\n")}")' 
JSON: 273, YAML: 196

These line savings also translate to more content fitting on the screen.

B) YAML has built-in comment support, which is important for documenting in place how and why certain changes were made without having to backtrack to a separate document.

C) Git diffs with YAML are more precise since it's not using commas for separation. For example, this commit shows one extra line being modified even though functionally only one line is being changed:

3d88cbb

Languages like Python will usually recommend treating commas as line terminators instead of as separators for this reason alone. You can't even use this formatting style in JSON:

>>> json.loads("{\"1\":[1, 2]}")
{'1': [1, 2]}
>>> json.loads("{\"1\":[1, 2,]}")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python3.12/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.12/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.12/json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 12 (char 11)

D) YAML in general is flexible with string formatting. You can more easily work with strings since you don't have to use escape characters as much. This can help with readability and also avoid bugs with quoted variables in script blocks and regexes in x-checker-data blocks.

I would say switching the build manifest to YAML would be a good clean break, however the point about maintaining continuity is still a great point. Perhaps we can go half-way by keeping the JSON build manifest via git mv com.spotify.Client.json com.spotify.Client.json.old?