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 2022/05/04 21:18:57 UTC

[GitHub] [james-project] mbaechler opened a new pull request, #986: Reworks MailRepository contract to use MailKeys instead of Mails

mbaechler opened a new pull request, #986:
URL: https://github.com/apache/james-project/pull/986

   Co-Authored-By: Matthieu Baechler <ma...@apache.org>


-- 
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: notifications-unsubscribe@james.apache.org

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


[GitHub] [james-project] mbaechler merged pull request #986: JAMES-3762 Reworks MailRepository contract to use MailKeys instead of Mails

Posted by GitBox <gi...@apache.org>.
mbaechler merged PR #986:
URL: https://github.com/apache/james-project/pull/986


-- 
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: notifications-unsubscribe@james.apache.org

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


[GitHub] [james-project] jeantil commented on pull request #986: JAMES-3762 Reworks MailRepository contract to use MailKeys instead of Mails

Posted by GitBox <gi...@apache.org>.
jeantil commented on PR #986:
URL: https://github.com/apache/james-project/pull/986#issuecomment-1118249908

   I have pushed a fix for the checkstyle violation, we will rebase -i and add the ticket number in the commit title once the CI clears. I didn't want to force push twice


-- 
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: notifications-unsubscribe@james.apache.org

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


[GitHub] [james-project] chibenwa commented on a diff in pull request #986: JAMES-3762 Reworks MailRepository contract to use MailKeys instead of Mails

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #986:
URL: https://github.com/apache/james-project/pull/986#discussion_r865484367


##########
server/mailrepository/mailrepository-api/src/test/java/org/apache/james/mailrepository/MailRepositoryContract.java:
##########
@@ -206,19 +206,19 @@ default void shouldPreserveDsnParameters() throws Exception {
         Mail mail = createMail(MAIL_1);
         mail.setDsnParameters(dsnParameters);
 
-        testee.store(mail);
+        var key = testee.store(mail);
 
-        assertThat(testee.retrieve(MAIL_1).dsnParameters()).contains(dsnParameters);
+        assertThat(testee.retrieve(key).dsnParameters()).contains(dsnParameters);
     }
 
     @Test
     default void retrieveShouldReturnNullAfterRemoveAll() throws Exception {
         MailRepository testee = retrieveRepository();
-        testee.store(createMail(MAIL_1));
+        var key = testee.store(createMail(MAIL_1));

Review Comment:
   I thought we once upon a time decided to not use `var` as it looses type explicitness.
   
   



##########
server/protocols/webadmin/webadmin-mailrepository/src/main/java/org/apache/james/webadmin/service/ReprocessingService.java:
##########
@@ -86,12 +86,12 @@ static class Reprocessor implements Closeable {
             this.configuration = configuration;
         }
 
-        private void reprocess(MailRepository repository, Mail mail) {
+        private void reprocess(MailRepository repository, Mail mail, MailKey key) {

Review Comment:
   If we have the mail we have the key? IE this method signature can likely stay the same.



-- 
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: notifications-unsubscribe@james.apache.org

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


[GitHub] [james-project] Arsnael commented on pull request #986: JAMES-3762 Reworks MailRepository contract to use MailKeys instead of Mails

Posted by GitBox <gi...@apache.org>.
Arsnael commented on PR #986:
URL: https://github.com/apache/james-project/pull/986#issuecomment-1118121620

   ```
   22:20:52,265 [INFO] There is 1 error reported by Checkstyle 8.29 with checkstyle.xml ruleset.
   
   22:20:52,265 [ERROR] src/main/java/org/apache/james/mailrepository/jdbc/JDBCMailRepository.java:[37,8] (imports) UnusedImports: Unused import - java.util.Collection.
   ```


-- 
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: notifications-unsubscribe@james.apache.org

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


[GitHub] [james-project] mbaechler commented on pull request #986: JAMES-3762 Reworks MailRepository contract to use MailKeys instead of Mails

Posted by GitBox <gi...@apache.org>.
mbaechler commented on PR #986:
URL: https://github.com/apache/james-project/pull/986#issuecomment-1120768790

   squash and add ticket number then push force


-- 
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: notifications-unsubscribe@james.apache.org

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


[GitHub] [james-project] Arsnael commented on pull request #986: JAMES-3762 Reworks MailRepository contract to use MailKeys instead of Mails

Posted by GitBox <gi...@apache.org>.
Arsnael commented on PR #986:
URL: https://github.com/apache/james-project/pull/986#issuecomment-1119345671

   ```
   07:21:12,685 [ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:check (check-style) on project james-server-mailrepository-memory: You have 1 Checkstyle violation. -> [Help 1]
   
   org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:check (check-style) on project james-server-mailrepository-memory: You have 1 Checkstyle violation.
   ```
   An other one... Couldn't see it in the logs this time though, might want to do some digging :)


-- 
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: notifications-unsubscribe@james.apache.org

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


[GitHub] [james-project] jeantil commented on pull request #986: JAMES-3762 Reworks MailRepository contract to use MailKeys instead of Mails

Posted by GitBox <gi...@apache.org>.
jeantil commented on PR #986:
URL: https://github.com/apache/james-project/pull/986#issuecomment-1119810549

   local build was green, I wonder if checkstyle could be moved from compile to validate: compiling james is pretty heavy ... 


-- 
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: notifications-unsubscribe@james.apache.org

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