Skip to content

Conversation

@l-trotta
Copy link
Contributor

@l-trotta l-trotta commented Jun 6, 2024

Requirements: summary should contain the first sentence of description, while description should contain the remaining phrases.

@l-trotta l-trotta marked this pull request as draft June 6, 2024 15:58
@l-trotta l-trotta requested a review from a team June 6, 2024 15:59
Copy link
Contributor

@swallez swallez left a comment

Choose a reason for hiding this comment

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

I left lots of nitpicking comments to improve your Rust-fu 😉

Copy link
Contributor

@Anaethelion Anaethelion left a comment

Choose a reason for hiding this comment

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

Left some comments, looks cleaner with the segmenter.
Some of @swallez comments might still apply.

Copy link
Contributor

@swallez swallez left a comment

Choose a reason for hiding this comment

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

LGTM once we handle the additional pathological case.

@l-trotta l-trotta marked this pull request as ready for review June 17, 2024 16:20
@l-trotta l-trotta merged commit 3120d62 into main Jun 17, 2024
@l-trotta l-trotta deleted the better-summary-oapi branch June 17, 2024 17:09
flobernd pushed a commit that referenced this pull request Jun 18, 2024
* test

* further test

* refactoring

* linter

* using segmenter to split sentences

* removing newlines + unit test

* comment

* handled specific case
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants