#41 Add default value in the ConfigProperty

[Emily-Jiang]

Signed-off-by: Emily Jiang [email protected]

[Emily-Jiang]

I'm going to merge this in and discuss one extra API I added getRefreshDuration this afternoon. The PR contains default value capability on the injection model.

[struberg]

+1 for the default value in ConfigProperty
-1 for the getRefreshDuration. As explained in the PR (open review question) it is not clear what use case it does solve.

[struberg]

Sorry Emily, Both @OndrejM and I expressed a strong -1 against getRefreshDuration during our yesterdays meeting. And there was also an open Review comment asking for the use case from 15h ago. Let's discuss this in our hangout today.

[Emily-Jiang]

That is what I explained in the above comment to discuss this afternoon. I'll detail the use cases in the hangout.

[OndroMih]

I don't like doing more than 2 different things in a PR. While I agree with kavadoc changes, the refresh duration discussion has not been closed yet.

[OndroMih]

While having a new default method shouldn't hurt, this one is very confusing. The javadoc explains the case of static source, but what should be returned in case of a dynamic one? It's not possible to guess how long a refresh would take. Wouldn't be enough to return just true/false for static/dynamic source? I don't see any point in guessing how fast would the source refresh...

[Emily-Jiang]

@OndrejM I think the compromise would be isDynamic to at least state it is dynamic or not. Anyway, I am trying to draft out a note and ask for a wide discussion.

[struberg]

Yes, the current method makes not much sense to me for now.

• There is no described use case
• It's nowhere referenced in any test
• It's nowhere implemented

Hard to tell what the intent is actually.