Skip to content
This repository was archived by the owner on Apr 13, 2025. It is now read-only.

Conversation

@ukstudio
Copy link
Contributor

Hi, there.

I implemented 'notification' resource and 'notifications' endpoint.
Please, give me feedback if this pull request has any problems

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.18%) when pulling 56609f4 on ukstudio:listing_notifications into 5b0af80 on dashofcode:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.18%) when pulling 56609f4 on ukstudio:listing_notifications into 5b0af80 on dashofcode:master.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can change this to be a project and story object since the API returns nested objects.

attribute :project, , TrackerApi::Resources::Project
attribute :story, , TrackerApi::Resources::Stroy
attribute :performer, TrackerApi::Resources::Person

Then you can remove the code where you manually pull out notification['story']['id'] and notification['project']['id'].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. I try it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix it. Please, check it.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.18%) when pulling 7c58d59 on ukstudio:listing_notifications into 5b0af80 on dashofcode:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.18%) when pulling 8c3c992 on ukstudio:listing_notifications into 5b0af80 on dashofcode:master.

@forest
Copy link
Contributor

forest commented Jan 15, 2015

Thanks. I'll get this merged in.

@ukstudio
Copy link
Contributor Author

Thanks!!

forest added a commit that referenced this pull request Jan 20, 2015
Implement TrackerApi::Client#notifications
@forest forest merged commit 45cd400 into irphilli:master Jan 20, 2015
@ukstudio ukstudio deleted the listing_notifications branch January 20, 2015 07:17
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