Skip to content

Relax the visibility of MockMVC DSL constructors - #33778

Closed
Hejow wants to merge 3 commits into
spring-projects:mainfrom
Hejow:feature/add-rest-docs-dsl
Closed

Relax the visibility of MockMVC DSL constructors#33778
Hejow wants to merge 3 commits into
spring-projects:mainfrom
Hejow:feature/add-rest-docs-dsl

Conversation

@Hejow

@Hejow Hejow commented Oct 23, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hello, I added Kotlin DSL for RestDocs.

At first, I thought this PR might be more suitable for spring-restdocs, so I was confused where to make PR.
Due to the internal constructor, I’m submitting the PR here.

I positioned the package with the following three considerations in mind

  1. The duplication issue that arises when placed in the same package as the existing Kotlin DSL.
  2. Preventing confusion for users.
  3. Reflecting the package structure of the existing Rest-docs module

I would appreciate any feedback on this PR. Thank You!!

@pivotal-cla

pivotal-cla commented Oct 23, 2024

Copy link
Copy Markdown

@Hejow Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla

pivotal-cla commented Oct 23, 2024

Copy link
Copy Markdown

@Hejow Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 23, 2024
@sdeleuze sdeleuze self-assigned this Oct 23, 2024
@sdeleuze sdeleuze added in: test Issues in the test module theme: kotlin An issue related to Kotlin support labels Oct 23, 2024
@sdeleuze

sdeleuze commented Oct 28, 2024

Copy link
Copy Markdown
Contributor

Adding RestDocs specific code in Spring Framework is not ok, I would favor relaxing the visibility of the constructor. Could you please refine the PR accordingly and add related test, or create a related issue and close this PR?

@sdeleuze sdeleuze added the status: waiting-for-feedback We need additional information before we can continue label Oct 28, 2024
@Hejow

Hejow commented Oct 29, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Thank you for ur feedback!
I guess it's much better to relaxing visibility of the constructor and let user to extend by themselves!
I will refine it! Thx 👍

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Oct 29, 2024
@Hejow
Hejow force-pushed the feature/add-rest-docs-dsl branch 2 times, most recently from d6b7cae to 31732ac Compare November 5, 2024 15:40
@Hejow
Hejow force-pushed the feature/add-rest-docs-dsl branch from 31732ac to a940570 Compare November 5, 2024 15:42
@Hejow

Hejow commented Nov 5, 2024

Copy link
Copy Markdown
Contributor Author

@sdeleuze Sorry that im lateee.. 😭
I refined this PR by relaxing visibility of mockMvc DSL constructor for users to customize themselves.
So, previously implemented Restdocument DSL is no longer needed, i removed it!

@sdeleuze

sdeleuze commented Nov 5, 2024

Copy link
Copy Markdown
Contributor

Could we use protected constructors instead of public ones?

@sdeleuze sdeleuze added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 5, 2024
@sdeleuze sdeleuze added this to the 6.2.0 milestone Nov 5, 2024
@Hejow

Hejow commented Nov 6, 2024

Copy link
Copy Markdown
Contributor Author

@sdeleuze I love to restrict visibility of the constructors.
But in this case, when we restrict them as protected each of DSL cannot access lamda. 🥹

p.s. thx for ur feedbacks!!! 👍

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 6, 2024
@sdeleuze

sdeleuze commented Nov 6, 2024

Copy link
Copy Markdown
Contributor

Ah yeah, indeed.

@sdeleuze sdeleuze changed the title Add RestDocumentation Kotlin DSL Relax the visibility of MockMVC DSL constructors Nov 6, 2024
@sdeleuze sdeleuze removed the status: feedback-provided Feedback has been provided label Nov 6, 2024
@sdeleuze sdeleuze closed this in 4697ae1 Nov 6, 2024
@sdeleuze

sdeleuze commented Nov 6, 2024

Copy link
Copy Markdown
Contributor

Squashed and merged, thanks!

@sdeleuze sdeleuze added the type: enhancement A general enhancement label Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@sdeleuze sdeleuze

Labels

in: test Issues in the test module theme: kotlin An issue related to Kotlin support type: enhancement A general enhancement

Projects

None yet

Milestone

6.2.0

Development

Successfully merging this pull request may close these issues.

4 participants

@Hejow @pivotal-cla @sdeleuze @spring-projects-issues