The following concerns/issues still need to be addressed for this PR:
1. With our changes, `FunctionService.getFunction(String)` will now return a 
`TimingFunction` instead of the function that that was passed to 
`FunctionService.registerFunction(Function)`. This could be a problem for users 
if they are doing an explicit cast after getting the function, like 
`(MyFunction) FunctionService.getFunction("MyFunction")`. Is this a breaking 
change? I have a thread going on the dev list about this topic.
2. The PR is failing `AnalyzeSerializablesJUnitTest.testSerializables` because 
we added a new Serializable class, `TimingFunction`. We need to determine if 
this class can actually be serialized. If it cannot, we just need to add it to 
`excludedClasses.txt` and we're done. If it can be serialized, we need to 
consider the implications. One possible way to avoid backwards compatibility 
issues would be to override the serialization behavior to replace the 
serialized form of `TimingFunction` with that of its inner function. After 
considering the implications, adding the class to 
`sanctioned-geode-core-serializables.txt` will fix the failing test.
3. Many D-unit tests are failing due to `ClassCastException` and 
`AssertionError` being thrown because we are now returning `TimingFunction` 
instead of the function that was passed to 
`FunctionService.registerFunction(Function)`. We verified that these exceptions 
are all thrown from test code and not product code. We need to update the test 
code to expect an instance of `TimingFunction` and know how to get the inner 
function from the `TimingFunction`.

[ Full content available at: https://github.com/apache/geode/pull/4038 ]
This message was relayed via gitbox.apache.org for 
notifications@geode.apache.org

Reply via email to