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

Tp 3 #346

Merged
merged 129 commits into from
Aug 3, 2019
Merged

Tp 3 #346

merged 129 commits into from
Aug 3, 2019

Conversation

dlemures
Copy link
Collaborator

No description provided.

…bindings to internal provider. Now bindings DSL is more clear: it can only ***add*** things to annotated bound classes. Annotations will always be taken into account, whether or not we use bindinds vs dynamic discovery. Bindings DSL can emulate having added annotations.
… well together. DSL is not yet fully tested. TODO: add the possibility to define in which scope a binding should go based on scope annotations
…orks well, we can now define scope very accurately in the DSL
…asses using a scope annotation can only be installed in scope supporting this annotation. It means that all bindings will always be scoped in the scope where they are installed.
@coveralls
Copy link

coveralls commented Jul 30, 2019

Coverage Status

Coverage decreased (-0.5%) to 95.192% when pulling 7f87a05 on tp-3 into 54476ca on master.

Copy link
Owner

@stephanenicolas stephanenicolas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also: Create a ticket for TP 3.1.0 where we add a base class for factories, in order to generate less code for factories, reduce the size of the final jar/apk. It will impact many tests.


class DelegateNotifier {

private val delegatesMap: MutableMap<Any, MutableList<InjectDelegate<out Any>>> = Collections.synchronizedMap(WeakHashMap())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We normally use the convention mapFooToBar. Here it would be mapContainerToDelegates

@@ -0,0 +1,25 @@
/*
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to make this more uniform. Sometimes we use Extension suffix with an s and sometimes not.

}

@Test
fun `openScope should open scope and wrap it using KTPScope`() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KTPScope ?

}

@Test
fun `openScope should open scope and wrap it using KTPScope and provide configuration`() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KTPScope

}

@Test
fun `openScopes should open scopes and wrap child scope using KTPScope`() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KTPScope

@@ -138,6 +138,7 @@ afterEvaluate { project ->
task androidJavadocs(type: Javadoc) {
source = android.sourceSets.main.java.source
classpath += project.files(android.getBootClasspath().join(File.pathSeparator))
excludes = ['**/*.kt']
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how can we make that work for kotlin too ?

import toothpick.configuration.Configuration
import toothpick.ktp.delegate.DelegateNotifier

class KTP : Toothpick() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Javadoc on all kotlin code !

if (modelClass.isAssignableFrom(TPViewModel.class)) {
return (T) new TPViewModel(scope);
}
throw new IllegalArgumentException("Unknown ViewModel class: " + modelClass.getName());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Not a view model class: "

@Override
protected void onCleared() {
super.onCleared();
closeScope(scope.getName());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inverse order

this.hasScopeInstancesAnnotation = hasScopeInstancesAnnotation;
this.hasSingletonAnnotation = hasSingletonAnnotation;
this.hasReleasableAnnotation = hasReleasableAnnotation;
this.hasProvidesSingletonInScopeAnnotation = hasProvidesSingletonInScopeAnnotation;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove InScope

/**
* Simple version of the BackpackItemsActivity.
*
* Here we do not retain the backpack at all.
Copy link
Owner

@stephanenicolas stephanenicolas Aug 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this example the backpack is not retained on configuration changes as it belongs to the activity scope which follows the lifecycle of activity instances: when an instance is destroyed, its associated scope is closed and a new scope with a new backpack will be created when the new instance of the activity is created.

/**
* Advanced version of the BackpackItemsActivity.
*
* Here we retain the backpack on configuration changes using the ViewModel Scope.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this example, the backpack is retained on configuration changes as it belongs to the view model scope which follows the lifecycle of view model instances: when an instance is destroyed, and later recreated, the scope remains unchanged and the backpack instance will be the same.

@dlemures dlemures merged commit 5da32be into master Aug 3, 2019
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.

None yet

3 participants