Conversation
chibenwa
left a comment
chibenwa
left a comment
There was a problem hiding this comment.
Thanks for hanging on.
I suspect a minor rebase issue. Feel free to disregard the comment regarding indent (it's a nitpick).
...res/src/main/java/org/apache/james/mailbox/postgres/PostgresMailboxSessionMapperFactory.java
Show resolved
Hide resolved
| new PostgresMessageDAO(executorFactory.create(session.getUser().getDomainPart()), blobIdFactory), | ||
| new PostgresMailboxMessageDAO(executorFactory.create(session.getUser().getDomainPart())), | ||
| getModSeqProvider(session), | ||
| getAttachmentMapper(session), | ||
| blobStore, | ||
| blobIdFactory, | ||
| clock); | ||
| new PostgresMessageDAO(executorFactory.create(session.getUser().getDomainPart()), blobIdFactory), | ||
| new PostgresMailboxMessageDAO(executorFactory.create(session.getUser().getDomainPart())), | ||
| getModSeqProvider(session), | ||
| getAttachmentMapper(session), | ||
| blobStore, | ||
| blobIdFactory, | ||
| clock); |
There was a problem hiding this comment.
The diff is full of reindents that makes it harder to read and harder to find what really changed.
There was a problem hiding this comment.
Thanks for the feedback.
I fixed the issue and reverted the unintended re-indentation to keep the diff minimal and focused on the actual change.
| this.mailbox = mailbox; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
We are missing the critical changes of mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/PostgresMessageManager.java where we actually chose which messageStorer we want: I suspect a rebase issue.
There was a problem hiding this comment.
Yes Rebase problem .
Arsnael
left a comment
Arsnael
left a comment
There was a problem hiding this comment.
LGTM, thanks fior the contribution @omerfarukicen !
|
There is still rebase conflicts though? (at least that's what github is saying to me) |
|
GitHub still reports rebase conflicts. However, I ran all tests locally and reviewed the changes, and I didn’t see any issues. |
|
Locally 
|
chibenwa
force-pushed
the
JAMES-4150-fixmaster
branch
from
dfa3c79 to
c016742
Compare
|
@omerfarukicen I forced pushed a locally rebased version, and I made sure to revert the indent changes that undermines this work. Is this fine to you? |
3 participants
JAMES-4150 Make Attachment Storage Optional for PostgreSQL Message Store #2885
This PR includes the changes that were previously implemented in the Postgres branch and have now been merged into the master branch.