You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2021/04/16 11:30:41 UTC
[GitHub] [james-project] chibenwa commented on a change in pull request #391: JAMES-XXXX Modularize TypeState
chibenwa commented on a change in pull request #391:
URL: https://github.com/apache/james-project/pull/391#discussion_r614765575
##########
File path: server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/TypeNameParserFactory.scala
##########
@@ -0,0 +1,13 @@
+package org.apache.james.jmap.change
+
+import javax.inject.Inject
+
+class TypeNameParserFactory @Inject() (setTypeName: Set[TypeName]) {
Review comment:
Injects needs to be performed on java.util.Set not to confuse Guice. You need then to convert it to a scala set.
##########
File path: server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/StateChange.scala
##########
@@ -40,33 +40,69 @@ object TypeName {
}
}
-sealed trait TypeName {
+trait TypeName {
def asMap(maybeState: Option[State]): Map[TypeName, State] =
maybeState.map(state => Map[TypeName, State](this -> state))
.getOrElse(Map())
def asString(): String
+ def parse(string: String): TypeName
Review comment:
I guess it can fail so the return type could be
```
Either[IllegalArgumentException, TypeName]
```
##########
File path: server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/TypeNameParserFactory.scala
##########
@@ -0,0 +1,13 @@
+package org.apache.james.jmap.change
+
+import javax.inject.Inject
+
+class TypeNameParserFactory @Inject() (setTypeName: Set[TypeName]) {
+ val ALL: Set[TypeName] = setTypeName
+
+ def parse(string: String): Either[String, TypeName] = {
Review comment:
Either[IllegalArgumentException, TypeName] ?
##########
File path: server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/TypeNameParserFactory.scala
##########
@@ -0,0 +1,13 @@
+package org.apache.james.jmap.change
+
+import javax.inject.Inject
+
+class TypeNameParserFactory @Inject() (setTypeName: Set[TypeName]) {
+ val ALL: Set[TypeName] = setTypeName
Review comment:
val all
##########
File path: server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/TypeNameParserFactory.scala
##########
@@ -0,0 +1,13 @@
+package org.apache.james.jmap.change
+
+import javax.inject.Inject
+
+class TypeNameParserFactory @Inject() (setTypeName: Set[TypeName]) {
Review comment:
Can we have unit tests for this class?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org