-
Notifications
You must be signed in to change notification settings - Fork 984
add toml store #812
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
add toml store #812
Conversation
|
this will probably change with the incoming go-toml v2. |
|
No, there’s definitely interest, sorry I missed this. If a new major version of the toml library is going to be released soon, I think it’s best to wait until then before merging this. |
|
no worries, I just wanted to know. |
|
I'm keen on this, I could see it being valuable for gitlab-runner configs! :) It does not seem "go-toml v2" is going to be finalised anytime soon. Last comment here was in eb and now it's almost May. Could we merge this as is and then shift to v2 in a year or when it's finalised? That way we are not blocked and can have toml support. |
|
We are at 2.0.alpha2, guestimating more than a couple months to get to v2 final. If we want to push for this one, we need a review first. |
|
@autrilla can we possibly push for a review sooner? It seems like we're still some while away from go-toml v2. Unless we expect the API to dramatically change, it might be valuable to get TOML support sooner rather than later. We've had to use a couple workarounds to get sops working with TOML and we'd love to see first-class support so that we can iterate and contribute improvements we've written locally |
|
This would be really great to get merged 🙏🏽 |
|
@autrilla I was waiting for the v2 to be out and replace this PR implementation but the document representation of the toml is not in the scope of v2 for now, so it's not really convenient to work for now. Hence, could this PR be reviewed first? |
cc2ee88 to
7bc3b77
Compare
|
Hello, any updates on this PR? Thanks. |
| <<<<<<< HEAD | ||
| gopkg.in/yaml.v3 v3.0.0-20210107172259-749611fa9fcc | ||
| gotest.tools v2.2.0+incompatible // indirect | ||
| ======= | ||
| gotest.tools v2.2.0+incompatible | ||
| >>>>>>> cc2ee88b7 (add toml store) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also update README.rst.
More specifically, the section Important information on types, replace :
YAML and JSON type extensions
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
``sops`` uses the file extension to decide which encryption method to use on the file
content. ``YAML``, ``JSON``, ``ENV``, and ``INI`` files are treated as trees of data, and key/values arewith:
YAML, JSON, ENV, INI and TOML type extensions
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
``sops`` uses the file extension to decide which encryption method to use on the file
content. ``YAML``, ``JSON``, ``ENV``, ``INI`` and, ``TOML`` files are treated as trees of data, and key/values are|
Is there any chance of this being merged at this point? |
|
I was the original author, started it and waited a long time to be merged. |
|
Hey, just so you know, it seems that BurntSushi/toml is still maintained. |
|
maybe it's worth revisiting the previous PR #533 as I was blocked by go-toml/v2 |
Resolves #4372. Since sops doesn't yet support toml (getsops/sops#812), I ignored it for `toml`.
|
Hi, is there any update on this PR? Just curious if it’s still being considered. |
Fixes #369.
replace PR #533 as:
Still not perfect, comments are not preserved, but most of it should be here.