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

Fix YAML multiline config for package names #358

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

widhalmt
Copy link
Member

  • Use '>-' instead of '>' for YAML multline when creating package names.

Currently I can't reproduce the error that is shown in the issue. Since it's a simple fix I provided a PR anyway but we need to test this more before we merge it.

The extra newline at the end of the string can cause JSON parsing errors as reported by @cbeaujoin-stellar .

This change will strip the newline character after the name

fixes #357

@widhalmt widhalmt added the bug Something isn't working label Jan 22, 2025
@widhalmt widhalmt self-assigned this Jan 22, 2025
@widhalmt widhalmt marked this pull request as draft January 22, 2025 16:06
@widhalmt
Copy link
Member Author

needs #360 to be merged before for checks to succeed.

* Use '>-' instead of '>' for YAML multline when creating package names.

The extra newline at the end of the string can cause JSON parsing errors as reported by @cbeaujoin-stellar .

This change will strip the newline character after the name

fixes #357
@tbauriedel tbauriedel force-pushed the fix/yaml-multiline-packagename-357 branch from b41a269 to 0d99a03 Compare January 31, 2025 15:43
@tbauriedel
Copy link
Member

@widhalmt is this still a draft or ready for review?

@widhalmt
Copy link
Member Author

widhalmt commented Feb 3, 2025

@tbauriedel I wanted to wait for the checks to run through. My problem with this PR is that it might solve a problem that was reported by a user but I can't reproduce the problem. So - when checks run through I guess we can merge it because as far as I see it does no harm and is indeed the "correct" way to handle this.

@tbauriedel
Copy link
Member

@widhalmt Maybe we should add more to the Issue Template.
Information about the OS where the ansible is running, python, ansible version, etc. could help us.

@tbauriedel
Copy link
Member

If you mark the PR 'ready' I can give you the approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] beats_metricbeat_package newline character issue
2 participants