You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2021/12/14 15:39:05 UTC
[GitHub] [sling-site] paul-bjorkstrand opened a new pull request #75: Update models docs documenting any-visibility constructors
paul-bjorkstrand opened a new pull request #75:
URL: https://github.com/apache/sling-site/pull/75
Also, added a subheadings to separate basic usage into two subsections.
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-site] kwin commented on pull request #75: Update models docs documenting member visibility allowed for the different types of injection
Posted by GitBox <gi...@apache.org>.
kwin commented on pull request #75:
URL: https://github.com/apache/sling-site/pull/75#issuecomment-994448975
@paul-bjorkstrand Thanks a lot.
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-site] kwin commented on a change in pull request #75: Update models docs documenting any-visibility constructors
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #75:
URL: https://github.com/apache/sling-site/pull/75#discussion_r768884960
##########
File path: src/main/jbake/content/documentation/bundles/models.md
##########
@@ -55,6 +57,35 @@ Constructor injection is also supported (as of Sling Models 1.1.0):
Because the name of a constructor argument parameter cannot be detected via the Java Reflection API a `@Named` annotation is mandatory for injectors that require a name for resolving the injection.
+Constructors may use any visibility modifier (as of Sling Models 1.5.0):
Review comment:
```suggestion
Constructors may use any visibility modifier (as of [Sling Models 1.5.0](https://issues.apache.org/jira/browse/SLING-8069)):
```
##########
File path: src/main/jbake/content/documentation/bundles/models.md
##########
@@ -55,6 +57,35 @@ Constructor injection is also supported (as of Sling Models 1.1.0):
Because the name of a constructor argument parameter cannot be detected via the Java Reflection API a `@Named` annotation is mandatory for injectors that require a name for resolving the injection.
+Constructors may use any visibility modifier (as of Sling Models 1.5.0):
Review comment:
Shouldn't we mention fields and methods here as well for completion? IMHO those never had any restrictions in terms of visibility....
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-site] paul-bjorkstrand commented on pull request #75: Update models docs documenting any-visibility constructors
Posted by GitBox <gi...@apache.org>.
paul-bjorkstrand commented on pull request #75:
URL: https://github.com/apache/sling-site/pull/75#issuecomment-993670949
@kwin - here it is.
I'm flexible on the subheadings. I just felt that the manifest part blended in too much with the constructor/class part. Happy to remove them if you feel they are unnecessary.
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-site] paul-bjorkstrand commented on a change in pull request #75: Update models docs documenting member visibility allowed for the different types of injection
Posted by GitBox <gi...@apache.org>.
paul-bjorkstrand commented on a change in pull request #75:
URL: https://github.com/apache/sling-site/pull/75#discussion_r769207216
##########
File path: src/main/jbake/content/documentation/bundles/models.md
##########
@@ -55,6 +57,35 @@ Constructor injection is also supported (as of Sling Models 1.1.0):
Because the name of a constructor argument parameter cannot be detected via the Java Reflection API a `@Named` annotation is mandatory for injectors that require a name for resolving the injection.
+Constructors may use any visibility modifier (as of Sling Models 1.5.0):
Review comment:
Updated with a lot more detail re: field and method injection
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-site] paul-bjorkstrand commented on a change in pull request #75: Update models docs documenting any-visibility constructors
Posted by GitBox <gi...@apache.org>.
paul-bjorkstrand commented on a change in pull request #75:
URL: https://github.com/apache/sling-site/pull/75#discussion_r768962662
##########
File path: src/main/jbake/content/documentation/bundles/models.md
##########
@@ -55,6 +57,35 @@ Constructor injection is also supported (as of Sling Models 1.1.0):
Because the name of a constructor argument parameter cannot be detected via the Java Reflection API a `@Named` annotation is mandatory for injectors that require a name for resolving the injection.
+Constructors may use any visibility modifier (as of Sling Models 1.5.0):
Review comment:
For fields, yes. Call me crazy, but while I was digging in, to try to replace reflection with method handles, I did not see method _injection_ anywhere. Everywhere I see method-injection-like code, it only worked with an interface, using the proxy. I am not sure if private methods in interfaces are proxyable, thus injectable.
I would be happy to be proven wrong, though.
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-site] kwin commented on a change in pull request #75: Update models docs documenting any-visibility constructors
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #75:
URL: https://github.com/apache/sling-site/pull/75#discussion_r768974671
##########
File path: src/main/jbake/content/documentation/bundles/models.md
##########
@@ -55,6 +57,35 @@ Constructor injection is also supported (as of Sling Models 1.1.0):
Because the name of a constructor argument parameter cannot be detected via the Java Reflection API a `@Named` annotation is mandatory for injectors that require a name for resolving the injection.
+Constructors may use any visibility modifier (as of Sling Models 1.5.0):
Review comment:
You are right with regards to methods. Let's just mention fields for now as well.
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-site] kwin commented on a change in pull request #75: Update models docs documenting any-visibility constructors
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #75:
URL: https://github.com/apache/sling-site/pull/75#discussion_r768885463
##########
File path: src/main/jbake/content/documentation/bundles/models.md
##########
@@ -55,6 +57,35 @@ Constructor injection is also supported (as of Sling Models 1.1.0):
Because the name of a constructor argument parameter cannot be detected via the Java Reflection API a `@Named` annotation is mandatory for injectors that require a name for resolving the injection.
+Constructors may use any visibility modifier (as of Sling Models 1.5.0):
Review comment:
Shouldn't we mention fields and methods here as well for completeness? IMHO those never had any restrictions in terms of visibility....
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-site] paul-bjorkstrand commented on a change in pull request #75: Update models docs documenting any-visibility constructors
Posted by GitBox <gi...@apache.org>.
paul-bjorkstrand commented on a change in pull request #75:
URL: https://github.com/apache/sling-site/pull/75#discussion_r769024819
##########
File path: src/main/jbake/content/documentation/bundles/models.md
##########
@@ -55,6 +57,35 @@ Constructor injection is also supported (as of Sling Models 1.1.0):
Because the name of a constructor argument parameter cannot be detected via the Java Reflection API a `@Named` annotation is mandatory for injectors that require a name for resolving the injection.
+Constructors may use any visibility modifier (as of Sling Models 1.5.0):
Review comment:
Will make the change and push a bit later today. Regarding method injection for concrete models - I could see that as an enhancement. I'll add a new ticket for that feature
Also, I think the code could use a little attention in `ModeAdapterlFactory`(I REALLY want to refactor it, but just need the time to sit down and understand everything).
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-site] kwin merged pull request #75: Update models docs documenting member visibility allowed for the different types of injection
Posted by GitBox <gi...@apache.org>.
kwin merged pull request #75:
URL: https://github.com/apache/sling-site/pull/75
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org