kirklund commented on pull request #6746: URL: https://github.com/apache/geode/pull/6746#issuecomment-906756523
> First off - I love the change to SanctionedSerializablesService. A lot cleaner! > > In the future, it would better not to bundle these unrelated changes together. The RestRegionAPIIntegrationTest and QueryCommandDUnitTestBase stuff really doesn't have anything to do with serialization service changes and shouldn't be in here. > > Edit: approved the changed - what I thought was a problem is in test code, not product. @upthewaterspout QueryCommandDUnitTestBase is actually one of the few (maybe only) DUnit tests that directly exercises the serialization filter in Geode with an expected failure. I only touched it because its subclasses started failing when I added sanctioned serializables support to geode-dunit. RestRegionAPIIntegrationTest does not have anything to do with the serialization filter, however it broke because of changes I made to geode-web-api's build.gradle file. After making changes to it for debugging and understanding what it was doing, I decided to include it since it was part of the "work" I had to do rather than jump through hoops to move it to a different branch. So, QueryCommandDUnitTestBase is actually part of the changes for serialization filtering. The second one was not, but it was included because it was part of a problem area (our REST code) that blocked the work for two days (see GEODE-9504 geode-web-api needs unit tests). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
