Skip to content

Conversation

vlada-dudr
Copy link
Contributor

1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):

fixes #1986

2. What is the scope of this PR (e.g. component or file name):

I replaced vendored serde_yaml with maintained fork of it - serde_yaml_ng, which looks like being used by quite some projects.

When running tests, the failed ones are exactly same as on main.

@vlada-dudr vlada-dudr force-pushed the migrate-to-serde_yaml_ng branch from 2a0a544 to 70abd0f Compare September 9, 2025 13:39
Signed-off-by: Vladimír Dudr <dudr@metrans.cz>
@vlada-dudr vlada-dudr force-pushed the migrate-to-serde_yaml_ng branch from 70abd0f to d737e39 Compare October 2, 2025 13:18
Signed-off-by: Vladimír Dudr <dudr@metrans.cz>
@vlada-dudr
Copy link
Contributor Author

Ok, so i fixed that failing test. It seems that serializer behaviour changed. Now it looks like it produces unquoted strings for yaml 1.1 booleans - string "on" serializes as on (previously it was 'on'). I guess it may be problematic when consumer of such string is yaml 1.1 compliant.

@Peefy Peefy merged commit 68b4062 into kcl-lang:main Oct 8, 2025
13 checks passed
@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 18272153633

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 22 of 34 (64.71%) changed or added relevant lines in 6 files are covered.
  • 12 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+1.9%) to 72.502%

Changes Missing Coverage Covered Lines Changed/Added Lines %
kclvm/runtime/src/value/val_yaml.rs 11 15 73.33%
kclvm/tools/src/vet/expr_builder.rs 0 8 0.0%
Files with Coverage Reduction New Missed Lines %
kclvm/sema/src/advanced_resolver/node.rs 1 84.97%
kclvm/ast/src/ast.rs 5 69.46%
kclvm/sema/src/core/symbol.rs 6 53.86%
Totals Coverage Status
Change from base Build 15341422212: 1.9%
Covered Lines: 50567
Relevant Lines: 69746

💛 - Coveralls

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.

kclvm no longer builds on rust 1.89 because of serde_yaml

3 participants