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

Add the Adapter for GoFiber V2 #357

Merged
merged 12 commits into from
Apr 11, 2021
Merged

Add the Adapter for GoFiber V2 #357

merged 12 commits into from
Apr 11, 2021

Conversation

realPy
Copy link
Contributor

@realPy realPy commented Oct 31, 2020

Implement the adapter and unit test for gofiber V2

@realPy
Copy link
Contributor Author

realPy commented Oct 31, 2020

The deepsource failed but the detected issue was not introduced by the new code
root@059b5fefc2f7:/go/src/go-admin# go vet examples/fasthttp/main.go

examples/fasthttp/main.go:71:26: call of eng.AddConfig copies lock value: github.com/GoAdminGroup/go-admin/modules/config.Config contains sync.RWMutex
root@059b5fefc2f7:/go/src/go-admin# go vet examples/gofiber/main.go

examples/gofiber/main.go:76:26: call of eng.AddConfig copies lock value: github.com/GoAdminGroup/go-admin/modules/config.Config contains sync.RWMutex
root@059b5fefc2f7:/go/src/go-admin#

Same error on the other examples :/

@realPy
Copy link
Contributor Author

realPy commented Dec 20, 2020

@chenhg5 Can you help me with this?

Can't understand why i have this error but i check all example with go vet fasthttp , echo , all have the same error:
examples/echo/main.go:70:26: call of eng.AddConfig copies lock value: github.com/GoAdminGroup/go-admin/modules/config.Config contains sync.RWMutex

So why , the deepsource failed on this PR and not the other example on the master?

I know you are busy but it will be great to support this PR. GoFiber is an amazon framework. I'll use go admin with gofiber and it work great 👍

@chenhg5
Copy link
Collaborator

chenhg5 commented Dec 27, 2020

@chenhg5 Can you help me with this?

Can't understand why i have this error but i check all example with go vet fasthttp , echo , all have the same error:
examples/echo/main.go:70:26: call of eng.AddConfig copies lock value: github.com/GoAdminGroup/go-admin/modules/config.Config contains sync.RWMutex

So why , the deepsource failed on this PR and not the other example on the master?

I know you are busy but it will be great to support this PR. GoFiber is an amazon framework. I'll use go admin with gofiber and it work great 👍

@realPy Thanks for your PR and sorry for replying late, cause I am really busy recently. I will pull your code and look into the question.

@realPy
Copy link
Contributor Author

realPy commented Dec 27, 2020

@chenhg5 Thanks, I can wait :) i have forked the project until you have time to review.
I think i will make some other PR ;)

@realPy
Copy link
Contributor Author

realPy commented Feb 20, 2021

@chenhg5 Catch the lock mutex problem.
The prototype of AddConfig in engine shoud be:
func (eng *Engine) AddConfig(cfg *config.Config) *Engine
because with func (eng *Engine) AddConfig(cfg config.Config) *Engine cfg is a copy of cfg and the mutex is "lock copy"

@realPy
Copy link
Contributor Author

realPy commented Mar 7, 2021

Hi @chenhg5 ,
Possibilities to integrate this pr?
I'll use it since 4 month with no problem 👍

@chenhg5 chenhg5 merged commit a7f5287 into GoAdminGroup:master Apr 11, 2021
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.

2 participants