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

Use Mail gem over RMail #127

Open
wants to merge 20 commits into
base: develop
Choose a base branch
from
Open

Use Mail gem over RMail #127

wants to merge 20 commits into from

Conversation

gauteh
Copy link
Member

@gauteh gauteh commented Aug 14, 2013

Tracking issue.

Move to the Mail gem (https://github.com/mikel/mail) over RMail. Previous and related issues:

The main difference from #61 is that the same message id (no safe id / ref) is used so no changes are made to the Index for now. Initial tests suggest that the migration from RMail to Mail can use the existing Index since the message ids are preserved.

@eMBee
Copy link
Contributor

eMBee commented Mar 13, 2014

@jamesotron and me took a stab at merging current develop with use-mail. we didn't have time to complete the work though, so i don't know if it was successful. a bunch of tests are failing, and it's not running, so there is more work to do.

@gauteh
Copy link
Member Author

gauteh commented Mar 13, 2014

Excellent! Anxious to see the result..

Conflicts:
	lib/sup.rb
	lib/sup/maildir.rb
	lib/sup/mbox.rb
	lib/sup/message.rb
	lib/sup/message_chunks.rb
	lib/sup/modes/edit_message_mode.rb
	lib/sup/util.rb
	sup.gemspec
@eMBee
Copy link
Contributor

eMBee commented Mar 22, 2014

i put a first cut of the merge at eMBee/sup@5e36249
i added a fix for one test and worked around a conflict with the Singleton module.

mail is using rubys Singleton and that seems to get preference when loading sup. since the interfaces are not compatible, this causes sup to fail.

i don't know what is the correct solution for this, so for now i just renamed sup's Singleton to avoid the conflict.

there is one test that still fails about the binary encoding. it expects that a binary encoding produces an error, as i guess rmail can't handle binary. but mail handles it just fine, so the test fails because there is no error. if i understand this correctly then i believe the test can be removed.

@gauteh
Copy link
Member Author

gauteh commented Mar 22, 2014

Thanks, I changed the test to do the right thing with Mail. SupSingleton is probably ok, it was confusing anyway.

@gauteh
Copy link
Member Author

gauteh commented Mar 22, 2014

Put your supsingleton patch in a separate pull-request (#271), we should merge all generic changes into develop.

@gauteh
Copy link
Member Author

gauteh commented Apr 22, 2014

Reasons for not using hashes for unique identifiers: http://stackoverflow.com/questions/22841666/email-deduplication

end

## takes a RMail::Message, breaks it into Chunk:: classes.
#def _message_to_chunks m, encrypted=false, sibling_types=[]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this is never used anywhere, can we please remove the dead code, it's distracting.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's just the old RMail code I used as template - it is still not completely implemented in the Mail version of message_to_chunks, I would prefer to see at least the new message_to_chunks have comments with what is missing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, fair enough :)

* develop:
  history: bundler and threadkilling
  travis: drop 2.1.0 testing
  fix travis. The rake task to build a gem is now 'rake build'
  sup.gemspec: Standarise the Gemspec
  Merge #300: sup-tweak-labels sync back unless specified otherwise
  sup-sync: error msg on :update
  thread_view_mode: Kill a message (and next, prev etc..)
  history: new :with_attachment
  Add new colormap for attachment sybol with sane fallback for existing colorschemes.
  prepare for 0.17.0
  history: bugfixes
  ctrl-n, ctrl-p: don't close message if there is no next one
  completion-mode: correctly set completion character
  async edit: history
  async edit: automatically launch async hook
  async: option for always async

Conflicts:
	sup.gemspec
@ZJvandeWeg
Copy link
Contributor

Does someone have a clue how far along this is exactly? Im going over the commits etc and to me it seems that;

  • Crypto module needs to be implemented
  • Tests need to be written
    Am I missing something?

@gauteh
Copy link
Member Author

gauteh commented Apr 9, 2015

Excerpts from ZJvandeWeg's message of April 9, 2015 15:22:

Does someone have a clue how far along this is exactly? Im going over the commits etc and to me it seems that;

  • Crypto module needs to be implemented
  • Tests need to be written
    Am I missing something?

I think I missed some of the message chunk types. Also, I'm not sure if
everything around drafts, forwarding etc is implemented.

@ZJvandeWeg
Copy link
Contributor

Forwarding seems to work, though saving drafts is broken as you suspected. For both; n = 1. Will look into this today.

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

Successfully merging this pull request may close these issues.

4 participants