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

Fixes #23332: Serde errors do not point to the correct line number #5226

Conversation

amousset
Copy link
Member

@amousset amousset commented Dec 1, 2023

https://issues.rudder.io/issues/23332

image

Solves probably 75% of the problem with a low effort.

The idea is to avoid parsing an untagged enum with serde (as it totally breaks error reporting), and use a plain struct instead, and then convert manually to the different kinds.
The higher level errors still do not point to a line in the source (but it was never the case, and would require heavy lifting, like replacing serde with a lower level YAML parser), but all reporting error line are now correct.

@amousset amousset requested a review from Fdall December 1, 2023 00:39
@amousset
Copy link
Member Author

amousset commented Dec 1, 2023

PR updated with a new commit

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/5226
-- Your faithful QA
Kant merge: "All our knowledge begins with the senses, proceeds then to the understanding, and ends with reason. There is nothing higher than reason."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/76935/console)

@amousset
Copy link
Member Author

amousset commented Dec 1, 2023

OK, squash merging this PR

@amousset amousset force-pushed the bug_23332/serde_errors_do_not_point_to_the_correct_line_number branch from 2898be0 to 88b2c8f Compare December 1, 2023 09:24
@amousset amousset merged commit 88b2c8f into Normation:branches/rudder/8.0 Dec 1, 2023
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants