Skip to content
This repository was archived by the owner on Nov 19, 2024. It is now read-only.

Conversation

@gangstertim
Copy link

Removes ambiguity from namely docs, renames LICENSE.md to fix spelling error

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling f6ff66c on timmil:improves_docs into 77cf11b on jonathanchrisp:master.

@jwarykowski
Copy link
Owner

Hey @TimMil,

Was just about to comment on the order of the callback args but I've seen you've added another commit. Just to note licence is the British spelling but happy to change to the American equivalent.

If possible can we change the doc example so it uses the profiles method, the send method is a helper method and shouldn't really be called directly although this is feasible.

Thanks for contributing, as soon as the changes have been done above I'll be happy to merge. Please can you also rebase the changes down to one commit.

@gangstertim
Copy link
Author

Ah, sorry, just saw this.

So after making some changes, I've realized that my use case was completely different here: I was using the send method to make posts to the events api, which profiles couldn't do for me.

The most recent PR I've submitted was made to make send more flexible, but maybe the real solution here is to leave send as is and build out a new function events for interfacing with the events api, which uses send. What are your thoughts on that?

And my apologies, I didn't realize Licence is a british spelling. As a notoriously bad speller myself, I just assume other people make typos too :)

@jwarykowski
Copy link
Owner

@TimMil - No problems, I can see why you would have done that. I've only just recently started on this project and need to add all the other endpoints, putting the events request in another method was part of the plan.

I've started work on some additional changes which relate to your second PR, would you mind holding off until I can get these changes in? Basically I think each API call should call a request verb function which then call down to the send function. I'll try and prevent users from calling these functions so only the API function can be used e.g. profiles, events etc.

I'm going to close your second PR until I can get these changes in. No worries about the spelling as in theory I should have been using the American equivalent anyhow 👍

@jwarykowski
Copy link
Owner

Hey @TimMil, I've raised a new PR #5 here, please can you update this PR if you get chance? Please see my previous comments. Please also note that options passed through the namely requests go straight through to request.js so we need to pass something like:

var options = {
  body: {
    limit: 1,
    after: '83809753-615b-44ee-914b-3821fe2ee7ae'
  }
}
namely.profiles(options, function(e,r,b) { 
  console.log(b);
});

@jwarykowski
Copy link
Owner

@TimMil - do you have time to update this PR?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants