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

Add leaderboard widget type #15

Merged
merged 3 commits into from
Feb 6, 2015
Merged

Conversation

chriskinsman
Copy link
Contributor

No description provided.

@danjenkins
Copy link
Owner

hi @chriskinsman , check out PR #13 which I must admit I just hadn't got round to merging in, do you think you can add anything of value to that PR? Or should I just merge in #13 - the benefit of that one that I can see is that it has documentation :) Thanks for the PR! I just need to get better at accepting PRs after feedback has been addressed, just been a tad busy recently

@chriskinsman
Copy link
Contributor Author

That one is missing the format and unit options. Let me update my PR with
docs and you can accept that one.

Thanks!

On Fri, Jan 30, 2015 at 12:01 PM, Dan Jenkins [email protected]
wrote:

hi @chriskinsman https://github.com/chriskinsman , check out PR #13
#13 which I must
admit I just hadn't got round to merging in, do you think you can add
anything of value to that PR? Or should I just merge in #13
#13 - the
benefit of that one that I can see is that it has documentation :) Thanks
for the PR! I just need to get better at accepting PRs after feedback has
been addressed, just been a tad busy recently


Reply to this email directly or view it on GitHub
#15 (comment)
.

@chriskinsman
Copy link
Contributor Author

Added documentation

On Fri, Jan 30, 2015 at 12:01 PM, Dan Jenkins [email protected]
wrote:

hi @chriskinsman https://github.com/chriskinsman , check out PR #13
#13 which I must
admit I just hadn't got round to merging in, do you think you can add
anything of value to that PR? Or should I just merge in #13
#13 - the
benefit of that one that I can see is that it has documentation :) Thanks
for the PR! I just need to get better at accepting PRs after feedback has
been addressed, just been a tad busy recently


Reply to this email directly or view it on GitHub
#15 (comment)
.

};

// Only add unit if specified
if(unit)
Copy link
Owner

Choose a reason for hiding this comment

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

sorry, can you put the { on the same line as the if statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

danjenkins added a commit that referenced this pull request Feb 6, 2015
Add leaderboard widget type
@danjenkins danjenkins merged commit 139895b into danjenkins:master Feb 6, 2015
@danjenkins
Copy link
Owner

This has been released onto npm [email protected]

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.

2 participants