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: add manifests.yaml_stream impl at kclvm runtime. #285

Merged
merged 1 commit into from
Nov 14, 2022

Conversation

Peefy
Copy link
Contributor

@Peefy Peefy commented Nov 7, 2022

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

  • N
  • Y

feat: #94

  • Add the runtime function to implement the customized output of YAML stream format manifests.
  • TODO: The corresponding implementation of the KCL UI layer includes function call and runtime impl integration and more UI test suites. @zong-zhe
  • TODO: More yaml encode options to impl. @Peefy

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

3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Other

4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):

  • N
  • Y

5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:

  • Unit test
  • Integration test
  • Benchmark (add benchmark stats below)
  • Manual test (add detailed scripts or steps below)
  • Other
  • unit test suites: kclvm/runtime/src/manifests/yaml.rs

6. Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

@Peefy Peefy added the runtime Issues or PRs related to kcl runtime including value and value opertions label Nov 7, 2022
@Peefy Peefy self-assigned this Nov 7, 2022
@coveralls
Copy link
Collaborator

coveralls commented Nov 7, 2022

Pull Request Test Coverage Report for Build 3458664862

  • 254 of 258 (98.45%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-29.9%) to 59.763%

Changes Missing Coverage Covered Lines Changed/Added Lines %
kclvm/runtime/src/value/val_yaml.rs 8 9 88.89%
kclvm/runtime/src/context/mod.rs 0 3 0.0%
Totals Coverage Status
Change from base Build 3438202557: -29.9%
Covered Lines: 20405
Relevant Lines: 34143

💛 - Coveralls

@ldxdl
Copy link
Contributor

ldxdl commented Nov 7, 2022

More UI test cases are needed with the new sys lib function.

@Peefy
Copy link
Contributor Author

Peefy commented Nov 7, 2022

More UI test cases are needed with the new sys lib function.

Right! The main change of this PR is the implementation of the runtime function. More language UI implementations and test cases need to be completed in another PR by @zong-zhe

@ldxdl
Copy link
Contributor

ldxdl commented Nov 7, 2022

More options to consider and support:

  • Separator: which separator to use between YAML documents (defaults to '---')
  • Indent: which kind of indentation to use when emitting (defaults to 2)
  • Width: The character width to use when folding text (defaults to 80)
  • UseFold: Force folding of text when emitting (defaults to false)
  • UseBlock: Force all text to be literal when emitting (defaults to false)
  • UseVersion: Display the YAML version when emitting (defaults to false)
  • UseHeader: Display the YAML header when emitting (defaults to false)
  • Encoding: Unicode format to encode with (defaults to :Utf8)

@Peefy
Copy link
Contributor Author

Peefy commented Nov 10, 2022

More options to consider and support:

  • Separator: which separator to use between YAML documents (defaults to '---')
  • Indent: which kind of indentation to use when emitting (defaults to 2)
  • Width: The character width to use when folding text (defaults to 80)
  • UseFold: Force folding of text when emitting (defaults to false)
  • UseBlock: Force all text to be literal when emitting (defaults to false)
  • UseVersion: Display the YAML version when emitting (defaults to false)
  • UseHeader: Display the YAML header when emitting (defaults to false)
  • Encoding: Unicode format to encode with (defaults to :Utf8)

This is a good suggestion. I have added some of the options, and some of them left todo because serde_yaml library does not support these capabilities, such as the indent option (Ref: dtolnay/serde-yaml#337)

Copy link
Contributor

@ldxdl ldxdl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Contributor

@chai2010 chai2010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Peefy Peefy merged commit 2d3202f into kcl-lang:main Nov 14, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Nov 14, 2022
@Peefy Peefy deleted the feat-yaml-manifests branch January 16, 2023 13:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
runtime Issues or PRs related to kcl runtime including value and value opertions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants