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

Change YAML multiline string style for comments #120

Open
sschuberth opened this issue Apr 18, 2023 · 7 comments
Open

Change YAML multiline string style for comments #120

sschuberth opened this issue Apr 18, 2023 · 7 comments

Comments

@sschuberth
Copy link
Member

I just realized that the | we're currently using for multiline curation comments adds a trailing newline. This becomes visible as \n as part of the resolved configuration in analyzer results:

  - provider:
      id: "DefaultDir"
    curations:
    - id: "Maven:com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava"
      curations:
        comment: "This library is empty as its version string suggests.\n"
        is_metadata_only: true

This is not so nice. Thus I'm proposing to change comments to use >- instead, which strip the final newline and also folds lines.

Opinions @fviernau, @mnonnenmacher?

@fviernau
Copy link
Member

I'd appreciate also inut from @MarcelBochtler as he wrote related code in helper-cli IIRC, like e.g.: oss-review-toolkit/ort@062dd6d

@MarcelBochtler
Copy link
Member

MarcelBochtler commented Apr 18, 2023

I think it was a conscious decision to use | instead of >- because the line breaks should be kept in case the comment is displayed somewhere.

AFAIK this is currently not being done:

  • Curations comments are not displayed anywhere
  • Resolutions don't adhere the \n when being displayed in the webapp report (It would be an improvement if they actually would):
    image

As the analyzer-result.yml file is not meant to be read by humans, I don't really see any benefits by removing this additional information.
However, I don't have a strong opinion about this.

@sschuberth
Copy link
Member Author

We could also use |- as a compromise which does not fold newlines, but strips the final newline.

@fviernau
Copy link
Member

fviernau commented Apr 18, 2023

Could someone shed a light on why we want to use new lines / line breaks at all?

@sschuberth
Copy link
Member Author

Could someone shed a light on why we want to use new lines / line breaks at all?

See above, what @MarcelBochtler wrote: You might want to have control over the formatting in which comments are display, e.g. if you have a lengthy explanation with paragraphs.

@sschuberth
Copy link
Member Author

We could also use |- as a compromise which does not fold newlines, but strips the final newline.

So, any objection to align on using |-?

@mnonnenmacher
Copy link
Member

No objections from me.

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

No branches or pull requests

4 participants