Dark Mode

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Reduce stack utilization of TestTemplateTestDescriptor::execute#4024

Merged
marcphilipp merged 3 commits intojunit-team:mainfrom
amaembo:amaembo-patch-1
Sep 25, 2024
Merged

Reduce stack utilization of TestTemplateTestDescriptor::execute#4024
marcphilipp merged 3 commits intojunit-team:mainfrom
amaembo:amaembo-patch-1

Conversation

Copy link
Contributor

amaembo commented Sep 25, 2024 *
edited by marcphilipp
Loading

Overview

Fixes issue #4020

I hereby agree to the terms of the JUnit Contributor License Agreement.

Definition of Done

  • There are no TODOs left in the code
  • Method preconditions are checked and documented in the method's Javadoc -- no method declarations changed
  • Coding conventions (e.g. for logging) have been followed
  • Change is covered by automated tests including corner cases, errors, and exception handling -- refactoring only change, I believe no new tests are necessary
  • Public API has Javadoc and @API annotations -- no public APIs changed
  • Change is documented in the User Guide and Release Notes -- should I write something there, or the change is minor and doesn't worth mentioning?

marcphilipp and sormuras reacted with thumbs up emoji
amaembo force-pushed the amaembo-patch-1 branch from 3be8b10 to c3ed3b3 Compare September 25, 2024 06:47
marcphilipp approved these changes Sep 25, 2024
Copy link
Contributor Author

amaembo commented Sep 25, 2024

Not sure about formatting. I've formatted the changed fragment using spotlessApply, even though it's inside the formatter:off section. Probably it worth removing formatter:off.

Copy link
Contributor

jbduncan commented Sep 25, 2024

Not sure about formatting. I've formatted the changed fragment using spotlessApply, even though it's inside the formatter:off section. Probably it worth removing formatter:off.

I think so too. From what I've observed of Spotless over the years, users like JUnit 5 use formatter:(off|on) because the bundled Eclipse formatter can't handle fluent chains like streams very well, so the comments are used as an escape hatch. Thus they should be safe to remove, if the team is happy of course. :)

Copy link
Member

marcphilipp commented Sep 25, 2024

Agreed, let's remove them

amaembo force-pushed the amaembo-patch-1 branch from 0655c7d to 906a13f Compare September 25, 2024 07:37
marcphilipp self-assigned this Sep 25, 2024
marcphilipp changed the title Reduce stack utilization by getting rid of outer Stream API call in TestTemplateTestDescriptor::execute Reduce stack utilization of TestTemplateTestDescriptor::execute Sep 25, 2024
marcphilipp merged commit 8491772 into junit-team:main Sep 25, 2024
amaembo deleted the amaembo-patch-1 branch September 25, 2024 07:55
Copy link
Member

marcphilipp commented Sep 25, 2024

@amaembo Thank you for your contribution! I polished it a bit in c29dbb2.
amaembo reacted with thumbs up emoji

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

Reviewers

marcphilipp marcphilipp approved these changes

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Reduce stack utilization by getting rid of outer Stream API call in TestTemplateTestDescriptor::execute

3 participants