Skip to content

Conversation

@miso-devel
Copy link
Contributor

@miso-devel miso-devel commented Nov 23, 2023

Related issue #3713

what I did

  • add runLengthEncode() test
  • add runLengthDecode() test

coverage change

https://app.codecov.io/gh/denoland/deno_std/commit/145c81a3c648f8c0d108f1517536cdfc0f487522/indirect-changes

@CLAassistant
Copy link

CLAassistant commented Nov 23, 2023

CLA assistant check
All committers have signed the CLA.

@miso-devel miso-devel marked this pull request as ready for review November 23, 2023 08:24
@miso-devel miso-devel requested a review from kt3k as a code owner November 23, 2023 08:24
Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

Thanks for this! Right direction. Just one suggestion for how runLengthTestCases is structured.

Comment on lines 6 to 22
const runLengthTestCases: [number[], { d: string; r: string }, string][] = [
[
[1, 2, 3, 4, 5],
{ d: "AQIDBAU=", r: "AQEBAQE=" },
"return expected value if input is normal value",
],
[
[1, 1, 1, 1],
{ d: "AQ==", r: "BA==" },
"return expected value if input includes an continuous value",
],
[
[],
{ d: "", r: "" },
"return expected value if input is empty",
],
];
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we able to turn this into an array of objects? It'd be a little easier to follow.

Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

LGTM. Nice work. Thank you.

@iuioiua iuioiua merged commit a53bbcc into denoland:main Nov 24, 2023
realpha pushed a commit to realpha/deno_std that referenced this pull request Nov 24, 2023
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.

3 participants