Skip to content

Conversation

@smortex
Copy link
Collaborator

@smortex smortex commented Jan 31, 2024

Fixes #10

syslog-ng/syslog-ng#4755 added a `name` keyword
that cause confusion for our simle script.

Ensure we are processing an actual `{"name": "string"}` and not
somthing like `{"name": {...}}` to find keywords.
@smortex smortex requested a review from alltilla as a code owner January 31, 2024 20:09
@smortex smortex force-pushed the fix-upgrade-action branch from e403f14 to c2ed2e8 Compare January 31, 2024 20:14
@smortex smortex marked this pull request as draft January 31, 2024 20:17
The diff mainly consist in a single long line of keywords where new
keywords are added and old keywords may be removed.  Reviewing this is
a pain.

Generate a readable diff in the PR body for easily spotting issues.
@smortex
Copy link
Collaborator Author

smortex commented Jan 31, 2024

Seems good now, see #13.

@smortex smortex marked this pull request as ready for review January 31, 2024 20:54
@alltilla
Copy link
Collaborator

alltilla commented Feb 1, 2024

Ah, now I understand which new option caused the problem:

"options": {
    "name": {
        "name": "name",
        "params": [
            [
                "<string>"
            ]
        ]
    },
    "metadata-url": {
        "name": "metadata-url",
        "params": [
            [
                "<string>"
            ]
        ]
    },
    "": {
        "name": null,
        "params": [
            [
                "<empty>"
            ]
        ]
    }
}

I think the db is correct, and we incorrectly assumed that a key-value pair with the key "name" will always have a string value.

Your fix is perfect, thank you!

@alltilla alltilla merged commit e7ec566 into main Feb 1, 2024
@smortex smortex deleted the fix-upgrade-action branch May 4, 2025 20:11
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.

Recent "Upgrade" CI runs fail

3 participants