Skip to content

Conversation

@nolastan
Copy link
Collaborator

This is a work-in-progress not ready for merging.

Went beyond scope of branch name :P

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note that to run tests you'll have to do foreman run bundle exec rspec spec. Would be great if there's a way to run foreman by default.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure if it's a good idea to create a remote object in the initializer...?

So Zohoho::Lead.new("foo@bar.com") would create a new Zoho lead. Would it be better to do Zohoho::Lead.new("foo@bar.com").save?

Copy link
Owner

Choose a reason for hiding this comment

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

This is looking pretty good. Looking forward to when it's working

One small point -- I wouldn't hard code the token into the into the initializer through the ENV hash. This is assuming a particular environment. In most other gems of this nature, the token is retrieved outside the gem and passed in through the initializer.

Regarding the question about creating a remote object in the initializer... When the connection object is created, it is just initialized -- no connection is made to the Zoho servers on creation. The remote connection is only made once the call method is invoked.
Kenton White

On Sep 19, 2012, at 8:59 PM, Stan notifications@github.com wrote:

In lib/zohoho/lead.rb:

@@ -0,0 +1,65 @@
+module Zohoho

  • require 'httparty'
  • require 'json'
  • require 'xmlsimple'
  • require 'date'
  • require 'open-uri'
  • class Lead
  • include HTTParty
  • attr_accessor :id, :owner_id, :owner_name, :first_name, :last_name, :email, :source
  • def initialize(email = nil, info = {})
  •  @conn = Zohoho::Connection.new 'CRM', ENV['TOKEN']
    
  •  if email
    
    Not sure if it's a good idea to create a remote object in the initializer...?


Reply to this email directly or view it on GitHub.

@ghost ghost assigned KentonWhite Sep 20, 2012
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.

3 participants