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.

Sign up for GitHub

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

Jump to bottom

Add new attributes to return for users and repositories #158

Merged
merged 2 commits into from
Jan 29, 2019
Merged

Add new attributes to return for users and repositories #158

merged 2 commits into from
Jan 29, 2019

Conversation

kenyonj
Copy link
Contributor

@kenyonj kenyonj commented Jan 29, 2019

This PR:

  • Adds hireable to the whitelisted attributes available on the owner hash.
  • Adds decorator methods for stargazers_count and forks_count to the Repository object.

Why?

  • This will allow users of this gem to access more useful information from users and repositories.

@kenyonj
Add new attributes to return for users and repos
362b0ae 
This commit:
- Adds `hireable` to the whitelisted attributes available on the `owner` hash.
- Adds decorator methods for `stargazers_count` and `forks_count` to the `Repository` object.

Why?
- This will allow users of this gem to access more useful information from users and repositories.
parkr
parkr approved these changes Jan 29, 2019
View reviewed changes
Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

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

This looks great to me, thank you!!!

@ashmaroli
Copy link
Member

@kenyonj Please adjust the changes so that tests pass locally at your end and on our CI.
Thank you.

@ashmaroli
Copy link
Member

@kenyonj I think you missed a test for checking the :hireable metadata.

@kenyonj
Copy link
Contributor Author

kenyonj commented Jan 29, 2019

I think you missed a test for checking the :hireable metadata.

@ashmaroli
I didn't see any other tests for the other user metadata so it wasn't immediately clear to me where that would fit in.

@parkr
Copy link
Member

parkr commented Jan 29, 2019

@ashmaroli I’ll make a PR for those user tests sometime tomorrow!

ashmaroli
ashmaroli approved these changes Jan 29, 2019
View reviewed changes
Copy link
Member

@ashmaroli ashmaroli left a comment

Choose a reason for hiding this comment

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

LGTM!

@benbalter benbalter merged commit f6308d0 into jekyll:master Jan 29, 2019
benbalter added a commit that referenced this pull request Jan 29, 2019
@benbalter
Update for #158
e0c6b5c
@parkr
Copy link
Member

parkr commented Jan 29, 2019 •
edited
Loading

Thanks, @benbalter! I'm working on the tests now for site.github.owner.

@benbalter
Copy link
Contributor

@parkr 🤘 If you tackle the tests, glad to help bump and release (if I remember how... 😄)

@parkr parkr mentioned this pull request Jan 29, 2019
Merged
@jekyll jekyll locked and limited conversation to collaborators Jan 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@parkr parkr parkr approved these changes

@ashmaroli ashmaroli ashmaroli approved these changes

Assignees
No one assigned
Labels
frozen-due-to-age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kenyonj @ashmaroli @parkr @benbalter @jekyllbot