phobos icon indicating copy to clipboard operation
phobos copied to clipboard

Add additional unit tests for appender string

Open GalaxyRider111 opened this issue 7 months ago • 4 comments

Adding some more additional unit test for string appender for the issue #8789 (https://github.com/dlang/phobos/pull/8789).

GalaxyRider111 avatar Apr 26 '25 10:04 GalaxyRider111

Thanks for your pull request and interest in making D better, @GalaxyRider111! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#10763"

dlang-bot avatar May 14 '25 09:05 dlang-bot

There seems to be more than just unit tests in this PR.

Herringway avatar May 14 '25 14:05 Herringway

  • Why does this PR contain both unit tests and functional changes, when the title and description only mention unit tests?
  • Why does this PR contain changes to both appender and std.container.array, when the title and description only mention appender?
  • What is the purpose of these changes? A PR description should explain why changes are being made, not just say what the changes are.

pbackus avatar May 14 '25 15:05 pbackus

  • Why does this PR contain both unit tests and functional changes, when the title and description only mention unit tests?
  • Why does this PR contain changes to both appender and std.container.array, when the title and description only mention appender?
  • What is the purpose of these changes? A PR description should explain why changes are being made, not just say what the changes are.

Ok so, I'm sorry everyone, I have made a mistake. Initially, I added more unit tests for this PR. Then, I wanted to contribue to another issue, one that involves adding support for move-only types, and I have been working on this, hence the changes from std/container/array.d. Unfortunately, I kept working on the same branch as the one from this PR, my mistake, sorry for confusion.

GalaxyRider111 avatar May 14 '25 22:05 GalaxyRider111

This PR has had a broken CI for 6+ months, please reopen if you plan to continue working on this.

LightBender avatar Oct 27 '25 02:10 LightBender