Skip to content
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

feat: allow multifile yaml #570

Merged
merged 10 commits into from
Mar 26, 2024

Conversation

holzgeist
Copy link
Contributor

related to #532

It allows to modify multifile yaml files, separated by ---.
If the input is a multifile, the value_path needs to start with an index, e.g. [0].path.to.value if the value should be changed in the first file

I'm not sure how to remove the as unknown as ... casts, my TypeScript is too rusty for this.
ContentNode has ContentNode[] in its definition, so I feel like this is supposed to work, but it doesn't.

@fjogeleit
Copy link
Owner

Hey, thanks I will take a look over the weekend.

@holzgeist
Copy link
Contributor Author

Hi,
Looks like I didn't test thoroughly enough. The resulting output is a YAML array, not a multifile..
Not sure how to handle this properly. I would need to return/remember if the input was multifile or not and pass that as dump option. Simply converting every top-level array into a multifile won't do ;)

Option 1: Not only return the parsed data, but also some options/flags that need to be passed to the dump invocation
Option 2: Convert the parser into actual classes, instantiate it and remember if the input was multifile during parsing and use this information during dumping

Option 2 would make more sense IMHO.

WDYT?

@fjogeleit
Copy link
Owner

You can try a class with some state but not that also the combinations of multifile and multiple changes should work. I know thats not that easy to achieve, thats one of the reason I do not implement this feature yet

@holzgeist
Copy link
Contributor Author

@fjogeleit does the new test multiple changes in multiple files, including multifiles do what you asked?

@fjogeleit
Copy link
Owner

Hey @holzgeist, I am on a conference this week but will check it out next week.

can you check the codestyle checks?

the missing pr stuff is probably not working correctly. Will check the tests manually.

looks good in the first place, thanks

@holzgeist
Copy link
Contributor Author

Thanks!

I ran prettier, tests and co. now. Hopefully it works

I even added some documentation to the readme 😄

@fjogeleit fjogeleit merged commit 451fb54 into fjogeleit:main Mar 26, 2024
5 of 9 checks passed
@fjogeleit
Copy link
Owner

thanks for your work

@holzgeist holzgeist deleted the feat-allow-multifile-yaml branch March 26, 2024 15:48
svardhin-hv pushed a commit to sede-open/yaml-update-action that referenced this pull request Jul 26, 2024
* feat: allow multifile yaml
* test: don't require leading $
* chore: run prettier
* fix: package
* fix: output multifile if input was multifile
* fix: run `npm run package`
* chore: run prettier
* chore: run npm package
* doc: add example in README.md
* fix: apply linter suggestion
svardhin-hv added a commit to sede-open/yaml-update-action that referenced this pull request Jul 29, 2024
* support quotingType config

Signed-off-by: Frank Jogeleit <frank.jogeleit@web.de>

* update dist

Signed-off-by: Frank Jogeleit <frank.jogeleit@web.de>

* dependency updates

Signed-off-by: Frank Jogeleit <frank.jogeleit@web.de>

* feat: allow force pushes (fjogeleit#555)

* feat: allow force option
* chore: add input to actions.yaml
* docs: add parameter
* fix: use getBooleanInput

* Handle PR exists as info instead failing

Signed-off-by: Frank Jogeleit <frank.jogeleit@lovoo.com>

* Upgrade to Node20

Signed-off-by: Frank Jogeleit <frank.jogeleit@lovoo.com>

* Update Tooling (fjogeleit#560)

* Update tooling

Signed-off-by: Frank Jogeleit <frank.jogeleit@lovoo.com>

* feat: allow multifile yaml (fjogeleit#570)

* feat: allow multifile yaml
* test: don't require leading $
* chore: run prettier
* fix: package
* fix: output multifile if input was multifile
* fix: run `npm run package`
* chore: run prettier
* chore: run npm package
* doc: add example in README.md
* fix: apply linter suggestion

* Update README.md (fjogeleit#580)

* fix CI pipeline errors

---------

Signed-off-by: Frank Jogeleit <frank.jogeleit@web.de>
Signed-off-by: Frank Jogeleit <frank.jogeleit@lovoo.com>
Co-authored-by: Frank Jogeleit <frank.jogeleit@web.de>
Co-authored-by: Lawrence Aiello <lawrence.aiello@laiello.com>
Co-authored-by: Frank Jogeleit <frank.jogeleit@lovoo.com>
Co-authored-by: holzgeist <holzgeist@users.noreply.github.com>
Co-authored-by: Fabio Kapsahili <68349146+fkapsahili@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants