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

Abstract Router interface for App and Group. #476

Merged
merged 3 commits into from
Jun 18, 2020
Merged

Abstract Router interface for App and Group. #476

merged 3 commits into from
Jun 18, 2020

Conversation

kiyonlin
Copy link
Contributor

Router interface defines all router handle interface includes app and group router.When dealing with sub-routers, users do not need to care about the specific implementation.

@kiyonlin kiyonlin requested a review from a team as a code owner June 17, 2020 07:11
@kiyonlin kiyonlin requested review from ReneWerner87 and Fenny and removed request for a team June 17, 2020 07:11
Copy link
Member

@Fenny Fenny left a comment

Choose a reason for hiding this comment

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

Could you fix the error thrown by the tests? Thanks!

@kiyonlin
Copy link
Contributor Author

Could you fix the error thrown by the tests? Thanks!

All tests passed on my machine before I created the pull request. I don’t know why it failed in the CI.

@Fenny
Copy link
Member

Fenny commented Jun 17, 2020

##[error]./app.go:31:16: cannot use &(App literal) (value of type *App) as Router value in variable declaration: wrong type for method Add
##[error]./group.go:13:16: cannot use &(Group literal) (value of type *Group) as Router value in variable declaration: wrong type for method Add

If you could add a test case that would be great 👍

@Fenny Fenny mentioned this pull request Jun 17, 2020
@kiyonlin
Copy link
Contributor Author

Sorry for using wrong way to ensure App and Group implement Router interface. I’ll add a test case for this.

@renanbastos93
Copy link
Member

renanbastos93 commented Jun 18, 2020

Why do you add an interface that's be not using?
So we must set this contract with the struct sure.
e.g.

type Router interface{
    Get()
}

type Routing struct {
    Router
}

@kiyonlin
Copy link
Contributor Author

@renanbastos93 This Router interface is for users who use fiber with sub-router.

e.g.

func setupRoutes(router fiber.Router) {
  router.Get("/path1", handler1);
  router.Post("/path2", handler2);
}

The router maybe an *App instance or a *Group instance.

@Fenny Fenny merged commit 74ca665 into gofiber:master Jun 18, 2020
@Fenny Fenny mentioned this pull request Jun 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants