Introduce a message repository #12

merged 14 commits into from Mar 23, 2012


None yet
4 participants

tomafro commented Mar 23, 2012

No description provided.

tomafro and others added some commits Mar 22, 2012

@tomafro tomafro WIP towards introduction of a message repository
Using the driver of disconnecting HTTP requests from IMAP, Chris, James and I have been working towards adding a message repository.  We've been moving slowly and deliberately, trying to introduce classes wherever possible and drive the code through tests.
@tomafro tomafro MessageImporter deserves its own file b9b93f6
@tomafro tomafro Add a very basic FileBasedMessageStore 18f0199
@tomafro tomafro Re-animate the walking skeleton.
Introducing a singletonish MessageRespository makes me feel a little uneasy but it was the simplest way I could think to tie everything together.  Also, I'm still unsure the role cucumber tests should/do play.  Here they are just a simple end-to-end test, which is a useful driver but perhaps not their best purpose?

Finally: 'Fix up look sharp. Don't make me bring the blitz out and get dark.'
@chrisroos-and-lazyatom chrisroos-and-lazyatom Remove GmailAccount.
Since we've removed the downloading of emails from the request cycle
this object is no longer required.

tomafro commented on .gitignore in 244e28e Mar 23, 2012

I think .powrc belongs in a global .gitignore, not in each project


chrisroos replied Mar 23, 2012

Yeah, I thought so too but @lazyatom made me put it in here.

chrisroos-and-lazyatom added some commits Mar 23, 2012

@chrisroos-and-lazyatom chrisroos-and-lazyatom Introduce a method to import messages given credentials.
We need to be able to import messages from an account via a high-level
method that's suitable for calling from (for example) a Rake task.

We've put this method in `MessageImporter`, but wanted to keep the tests
slightly separate because we weren't 100% sure that it belonged there.

Having written the tests, and with the two `require`s that we've had to
add to `message_importer.rb`, we're now confident that it doesn't belong
in that class; we'll extract it into something else next.
@chrisroos-and-lazyatom chrisroos-and-lazyatom Introduce AccountMessageImporter.
As mentioned in 623566, we don't think that the MessageImporter should
be responsible for instantiating an IMAP connection and importing
messages into the repository.
@chrisroos-and-lazyatom chrisroos-and-lazyatom Introduce messages:import rake task.
We've questioned whether loading the Rails environment is sensible,
given the overhead of doing so. We'll discuss this separately.

tomafro commented on 6235665 Mar 23, 2012

I'm interested that you needed to add the requires, given lib is now on the load path. I agree that the increase in dependencies is a smell though.


lazyatom replied Mar 23, 2012

Yeah, this was me being overzealous, and I hadn't spotted that you'd added lib to the autoload paths. I am interested in exploring not using autoloading though - my plan is to explore this is a separate very-short-lived branch.

chrisroos-and-lazyatom added some commits Mar 23, 2012

@chrisroos-and-lazyatom chrisroos-and-lazyatom Remove .powrc from .gitignore.
It should be up to an individual developer to ignore .powrc, as @tomafro
points out[1]

@chrisroos-and-lazyatom chrisroos-and-lazyatom Add 'whenever' to the Gemfile.
We're planning to use the whenever gem to generate a crontab that
schedules the execution of our message import task.

We'll test the capistrano integration locally before adding that.

After deploying this branch to linode we discovered that File.write is a Ruby 1.9.3 method - it's not defined in 1.9.2 which is what we currently have on the server. We're installing Ruby 1.9.3 on the server as I type...


tomafro replied Mar 23, 2012

Sweet! That's odd though - I'd have assumed it had been around forever. I just guessed it existed - seemed the natural interface to write a File.


tomafro commented on 4b2af9a Mar 23, 2012

Do you think this might be a bit premature? I'd be happy using manual imports until sauron actually becomes useful.


lazyatom replied Mar 23, 2012

We um'd and ah'd about how exactly to do this, but I do believe that it's important that the import happens automatically; the app was previously in the state that new emails would automatically appear, and I don't think we should regress unless absolutely necessary.

Whether or not we use whenever or something else though, I don't mind at all.

tomafro and others added some commits Mar 23, 2012

@tomafro tomafro Updated recap to try and prevent \r characters appearing in .env files 65cb943
@chrisroos-and-lazyatom chrisroos-and-lazyatom Disable Rails' asset pipeline.
We want to run the app in the production environment on linode but the
asset pipeline is causing us problems because it's expecting
pre-compiled assets to exist, yet recap isn't configured to generate

We're not using any CSS at the moment so the simplest thing to do is
disable it.
@chrisroos-and-lazyatom chrisroos-and-lazyatom Fix the build!
Disabling the asset pipeline broke three of our controller tests. It
would appear that the asset pipeline rack app which is mounted makes
Rails believe that there is a route which will point at the test
@chrisroos-and-lazyatom chrisroos-and-lazyatom Add `whenever` tasks to Capfile.
We've taken these `whenever` tasks from Harmonia, which in turn took them (albeit in a stripped down form) from the whenever capistrano recipes.

chrisroos merged commit 08ea109 into master Mar 23, 2012

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