Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(): interface improvement for discovery service #10885

Closed

Conversation

Tony133
Copy link
Contributor

@Tony133 Tony133 commented Jan 15, 2023

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Proposal:

  • Moved the DiscoveryOptions interface to a separate file, now inside the interfaces folder.
  • Created index.ts file inside interfaces folder.
  • Update the DiscoveryService

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3d183d94-d740-4510-b47c-24ec5156a3e2

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.409%

Totals Coverage Status
Change from base Build 43c8609d-7c9b-40c5-85e1-094c9c517952: 0.0%
Covered Lines: 6207
Relevant Lines: 6645

💛 - Coveralls

@kamilmysliwiec
Copy link
Member

This introduces a breaking change

@Tony133
Copy link
Contributor Author

Tony133 commented Jan 16, 2023

I thought so 😄, in fact I have others to propose similar to this one, but I waited to do it in PR, to see what you thought. Maybe I could group them in this PR? 🤔 I just have to rename the title the PR

But for the next major version of NestJS, this could be ok? right?

@kamilmysliwiec
Copy link
Member

We should only introduce breaking changes when it's absolutely necessary.

@nestjs nestjs locked and limited conversation to collaborators Jan 23, 2023
@Tony133 Tony133 deleted the refactor/move-interface-in-discovery branch February 2, 2023 13:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants