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

Replace wrapper hack with simple inheritance #7

Merged
merged 1 commit into from
Oct 31, 2014

Conversation

jkrems
Copy link
Collaborator

@jkrems jkrems commented Oct 31, 2014

This changes the way that gofer works internally while exposing the same interface. It also allows a new, simpler way of using gofer:

// ES6
import { Gofer, registerEndpoints } from 'gofer';

class Github extends Gofer {
}
// Because ES6 classes do not support value properties...
Github.prototype.serviceName = 'github';
Github.prototype.serviceVersion = '1.3.2';

// or just: Github.prototype.registerEndpoints({ ... })
registerEndpoints(Github, {
  emoji: function(request) { /* ... */ }
});
# coffee-script
{ Gofer } = require 'gofer'

class Github extends Gofer
  serviceName: 'github'
  serviceVersion: '1.3.2'

Github::registerEndpoints {
  emoji: (request) -> # ...
}

cb err ? data.error, data.result, responseData, response
module.exports = buildGofer = (serviceName, serviceVersion) ->
class CustomGofer extends Gofer
constructor: (config, hub) ->
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This could be removed when switching to POCS*.

[*] Plain old coffee-script, as opposed to CSR/redux.

@abloom
Copy link

abloom commented Oct 31, 2014

This seems reasonable. The diff is a little hard to read but I think it makes sense. The only oddity I noticed was the use of both Object.prototype.something and Object::something. Do you mind unifying on a single style?

@jkrems
Copy link
Collaborator Author

jkrems commented Oct 31, 2014

Went with all Object::something, updated.

@abloom
Copy link

abloom commented Oct 31, 2014

so fresh and so clean :shipit:

jkrems added a commit that referenced this pull request Oct 31, 2014
Replace wrapper hack with simple inheritance
@jkrems jkrems merged commit 09884e6 into master Oct 31, 2014
@jkrems jkrems deleted the jk/why-not-just-inherit branch October 31, 2014 19:47
@jkrems
Copy link
Collaborator Author

jkrems commented Oct 31, 2014

v1.2.0

@abloom
Copy link

abloom commented Oct 31, 2014

Can you open a new PR to update API.doc, README and examples/github.js?

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

2 participants