Update kitchen_sink_config.yml #2262
Conversation
updated kitchen sink to reflect letterboxd
|
The position of a lot of things are different, which makes review a little more difficult. |
| playlist_files: | ||
| - default: playlist | ||
| default: playlist |
| authorization: | ||
| access_token: (redacted) | ||
| token_type: Bearer | ||
| expires_in: 7889238 | ||
| expires_in: '7889238' |
There was a problem hiding this comment.
are quoted strings valid here?
|
Kometa put the quotes strings into my config.yml file.
Kometa does not seem to bark at the missing dash and the config-schema.json in vsc does not bark at it either.
I don't know what to say....
________________________________
From: Chaz Larson ***@***.***>
Sent: Wednesday, October 9, 2024 5:57:41 PM
To: Kometa-Team/Kometa ***@***.***>
Cc: bullmoose20 ***@***.***>; Author ***@***.***>
Subject: Re: [Kometa-Team/Kometa] Update kitchen_sink_config.yml (PR #2262)
@chazlarson requested changes on this pull request.
________________________________
In json-schema/kitchen_sink_config.yml<#2262 (comment)>:
playlist_files:
-- default: playlist
+ default: playlist
missing dash
________________________________
In json-schema/kitchen_sink_config.yml<#2262 (comment)>:
authorization:
access_token: (redacted)
token_type: Bearer
- expires_in: 7889238
+ expires_in: '7889238'
are quoted strings valid here?
—
Reply to this email directly, view it on GitHub<#2262 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AC7XXKNIOFFBCOBS726RKC3Z2WRFLAVCNFSM6AAAAABPVHGSYCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGNJYGM4TOMJSGI>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
I ask about the quotes because I've never had Kometa add them, so maybe there was some change in Kometa. The missing dash possibly works because perhaps a list with one item doesn't need them. I'm just going by the docs, which shows the dash in there. That line is showing as changed in this PR, as are the added quotes, for no apparent reason, so I'm asking about them. This PR is in theory only adding two lines, and there are a lot of things moving around and changing in this file. |
Ok... so let me restart to give us background and then we can update things accordingly. My new kitchen sink config is based on the output of QS. QS, does not implement libraries and overlay section so that's pretty much the same. So I took the output of QS which includes the playlist section and merged it with my original kitchen sink and ran Kometa against it. The config.yml file worked so I then took that and pasted the redacted version of the new config over the one in GH. Indeed, quotes ended up in the new version that were not there but Kometa added. Indeed the playlist section is missing a dash and indentation is off but the results worked. I also thought of reviewing what the yaml and our schema would say about this new kitchen sink config and it was fine with everything except the missing letterboxd in two places that I added. No complaints on the quotes or on the indentation/missing dash. Taking all of this, and looking at our wiki, it definitely deviates so we should decide on how to best move ahead. Option 1(Leaving the wiki alone): Option 2(leave QS output of playlist as is) I think option 1 is least impacting and since QS is not even mvp, we can fix that to output proper playlist section in MVP. Thoughts? |
|
I think the wiki should stay as it is, since this: takes a list, and if there is more than one item in that list the dashes are required. I don't think there's any reason to complicate matters by noting the one case in which they aren't. It could be that ruamel's YAML generation can be tweaked to always add that to lists; QS isn't directly creating any YAML. My personal opinion is that changes to files like this should be minimal, comprising only what's required. In this case, the only requirement is the addition of the two letterboxd lines, not changing all the headers and reordering things. I can see replacing these files with the output of QS when QS can generate a complete config, but I don't see the reason to do so today. That said, this is just a test file. |
|
I agree. Can you kill this commit? We can just add the two letterboxd default lines to the file. I can re-add the two lines but I don't want to mess up with the delete of this commit. |
updated kitchen sink to reflect letterboxd
Description
updated kitchen sink to reflect letterboxd
Issues Fixed or Closed
Type of Change
Please delete options that are not relevant.
Checklist
Please delete options that are not relevant.