Allowing the support for missing keys in Toml

[dcoraboeuf]

This PR proposes the support for missing keys when targeting TOML files.

Use case:

• the key containing the version is in a Toml file but does not necessarily exist
• upon an update, we want this key to be created with the new version

Today, this fails because updatecli requires the key to be already there.

By adding allowMissingKey: true to the spec, the target won't fail but just create the key when missing. By default, allowMissingKey remains false, keeping the initial behavior (raising an error about the missing key).

Two tests have been added:

• checking the legacy behaviour (without allowMissingKey)
• checking the creation of the key when allowMissingKey is true

Test

To test this pull request, you can run the following commands:

cp pkg/plugins/resources/toml
go test

Additional Information

Potential improvement

I did not find the documentation in the updatecli repository. Where can I edit the documentation?

[dduportal]

Hello 👋 Thanks for this contirbution!
I'll defer to @olbak for the final review / merge but I'll take care of giving a first review.

At firs glance:

• The use case is legit, it makes sense for me and the implementation looks good (at first sight)

• You can find the documentation in the following repository: https://github.com/updatecli/website (I assume https://github.com/updatecli/website/blob/master/content/en/docs/plugins/resource/toml.adoc). Thanks for asking! We can definitively improve the PR template and contribution doc to help contributors on this

• I've approved the GitHub checks (as it is your first contribution to this repository, GitHub does not mark you as "trusted" yet) so you'll have a feedback from the CI system

  • The spell check checker introduced in Add check-spelling #1340 (e.g. recently) points at occurences of non existing. I'm not sure if it is able to suggest a correction but I guess non-existing should work
  • The "Go / Build" chck fails at the golangci-lint step with the message Error: File is not gofmt-ed with -s (gofmt) => you should be able to fix it with the suggested command. You can try and validate on your environment with make lint which calls golangci-lint

[dcoraboeuf]

@dduportal , thanks for the review. I've pushed spelling corrections & ran gofmt. I'll prepare a PR for the documentation later today.

[dcoraboeuf]

I've created the documentation PR at #1377

I'm not familiar with the macros being used, but I think the list of parameters (including the new one) should be taken from the code directly.

However, I've fixed the Toml example since the keys should start with ..

[olblak]

Thanks @dcoraboeuf for the PR, and @dduportal for your review
I can merge the PR once the comment uses the corrent comment syntax

[hervelemeur]

Small nit after a quick glance, I'm wondering if createMissingKey could fit better than allowMissingKey, WDYT?

[dduportal]

Small nit after a quick glance, I'm wondering if createMissingKey could fit better than allowMissingKey, WDYT?

Good idea. Is that ok for you @dcoraboeuf ?

[olblak]

I've created the documentation PR at #1377

I'm not familiar with the macros being used, but I think the list of parameters (including the new one) should be taken from the code directly.

However, I've fixed the Toml example since the keys should start with ..

Indeed we generated a jsonschema based on the code (and its comments) and then that jsonschema is used to generate the website

[olblak]

File is not gofmt-ed with -s (gofmt)

My IDE is configured to do it automatically when I save a file

[dcoraboeuf]

Small nit after a quick glance, I'm wondering if createMissingKey could fit better than allowMissingKey, WDYT?

Good idea. Is that ok for you @dcoraboeuf ?

No problem. I'll provide a commit soon.

[dcoraboeuf]

I've changed AllowsMissingKey into CreateMissingKey