Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

James 1860 refactor #485

Closed
wants to merge 7 commits into from
Closed

Conversation

chibenwa
Copy link
Member

We can vote for the following refactors :

  • 1: Extracting (and test) flags operations
  • 2: Extracting (and test) mimeMessage parsing
  • 3: Rely on a provider for PERMANENT FLAGS
  • 4: MimeMessageReader should return a message

I vote for 1, 2, 3.

I downvote 4 as it implies a lot of copies. Or changing Message to hold a ShareInputStream (which also implies copy). I'm in favor of saving perfs, at the cost of "yet an other Message like class" that is not a Message because it has a SharedInputStream. Any comment welcomed.

@chibenwa chibenwa force-pushed the JAMES-1860-refactor branch 3 times, most recently from f5bf104 to b353275 Compare November 23, 2016 08:58
@chibenwa
Copy link
Member Author

test this please

2 similar comments
@chibenwa
Copy link
Member Author

test this please

@chibenwa
Copy link
Member Author

test this please

@chibenwa
Copy link
Member Author

chibenwa commented Nov 29, 2016

Error seems linked to an unmerged pull request :

09:19:03.732 [ERROR] o.a.j.m.i.c.CamelCompositeProcessor - Unable to init mailet Metrics: javax.mail.MessagingException: Can not load mailet Metrics;
  nested exception is:
	java.lang.ClassNotFoundException: org.apache.james.transport.mailets.Metrics

How is that possible ?

(https://james.open-paas.org/job/build%20pull-requests/670/)

Looks like we end up with configuration from an other PR.

My hint about that :

  • We use the same workspace
  • Our build is dockerized, hence conflict free
  • But we use the same local folders for :
    • Copying binaries
    • And config files

We need to prefix such directories with buildId.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant