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

Consolidate gs deletes #567

Conversation

ilkercelikyilmaz
Copy link
Contributor

Remove allocationmutex lock/unlock from Delete calls since we do have Shutdown state.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: e15e0775-9266-40f0-872e-978630a73ba5

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 91602979-5117-4483-b0cb-e90b68afbb78

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@aLekSer
Copy link
Collaborator

aLekSer commented Feb 7, 2019

Hello @ilkercelikyilmaz,
I think it is needed to squash commits into one before the merge happens. Please, refer to Contributing Guide.
Also it would be useful to run make test from your /build folder to run tests locally.
Lots of tests are failing cause of the change. For instance:

--- FAIL: TestSyncGameServerSet (0.24s)
    --- FAIL: TestSyncGameServerSet/adding_and_deleting_unhealthy_gameservers (0.12s)
        <autogenerated>:1: 
            	Error Trace:	controller_test.go:320
            	Error:      	Should be true
            	Test:       	TestSyncGameServerSet/adding_and_deleting_unhealthy_gameservers
            	Messages:   	A game servers should have been deleted
    --- FAIL: TestSyncGameServerSet/removing_gamservers (0.12s)
        <autogenerated>:1: 
            	Error Trace:	controller_test.go:345
            	Error:      	Not equal: 
            	            	expected: 5
            	            	actual  : 0

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.

3 participants