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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to