-
Notifications
You must be signed in to change notification settings - Fork 41
Conversation
27dbec2
to
2b6a869
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@durandom - The Activity stream addition (itself) looks good - though I think I would rather see the options change in Collection.rb as a separate PR.
2b6a869
to
b8b04e0
Compare
Checked commit durandom@b8b04e0 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 |
let(:raw_url_collection) { build(:response_url_collection, :klass => described_class, :url => url) } | ||
let(:raw_instance) { build(:response_instance, :group, :klass => described_class) } | ||
|
||
include_examples "Collection Methods" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shared example "Crud Methods" could be included in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid this endpoint only allows GET, HEAD, OPTIONS per
https://ansible_tower3/api/v1/activity_stream/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can split the "Crud Methods" shared example into multiple shared examples (Create, Update, Delete, etc.).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/ansible/ansible_tower_client_ruby/blob/master/spec/support/shared_examples/crud_methods.rb only has an example for 'POST'
Do you want shared examples for read in this PR? I'd rather like to do it in follow up, as I need this to move further for the event catcher in miq
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we are not 'POST'ing to this endpoint, we can deal with it later.
👍 LGTM |
also adds ability to pass down options to collections