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

[Ready for review] Enable updateResourceAsync #1085

Merged
merged 10 commits into from
Sep 22, 2016
Merged

[Ready for review] Enable updateResourceAsync #1085

merged 10 commits into from
Sep 22, 2016

Conversation

anuchandy
Copy link
Member

@anuchandy anuchandy commented Sep 16, 2016

  1. CreateableTaskGroup is renamed to createUpdateTaskGroup as the group contains task that can run update action on a resource

  2. Renamed ResourceCreator to ResourceCreatorUpdator and it exposes new method updateResourceAsync

  3. CreatableUpdatableImpl now contains default implementation of CreateAsync and UpdateAsync

  4. CreatableImpl inherites from CreatableUpdatableImpl and adding a new base class AppliableImpl that inherits from CreatableUpdatableImpl

    This will enable a resource which is only creatable to extends from CreatableImpl and anything that is applicable only to derive from ApplicableImpl.

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@azuresdkci
Copy link
Contributor

Runtime changes detected. pull request created. CI running: Build Status

@azuresdkci
Copy link
Contributor

Runtime changes detected. pull request created. CI running: Build Status

@anuchandy anuchandy changed the title [Ready for review - Testing progress, don't merge] Enable updateResourceAsync [Ready for review] Enable updateResourceAsync Sep 20, 2016
}

@Override
public final Observable<FluentModelT> createAsync() {
Copy link
Contributor

Choose a reason for hiding this comment

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

But createResourceAsync() still needs to be implemented?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, please add

@Override
public abstract Observable<FluentModelT> updateResourceAsync();

to reinforcement the implementation of update.

Copy link
Member Author

Choose a reason for hiding this comment

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

nice, will add them..

@azuresdkci
Copy link
Contributor

Runtime changes detected. pull request created. CI running: Build Status

@anuchandy
Copy link
Member Author

@jianghaolu addressed your comments, please take a look when you get chance.

Copy link
Contributor

@jianghaolu jianghaolu left a comment

Choose a reason for hiding this comment

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

LGTM

@anuchandy
Copy link
Member Author

merging as changes are approved.

@anuchandy anuchandy merged commit 243a323 into Azure:master Sep 22, 2016
@anuchandy anuchandy deleted the dag-task-rework branch February 8, 2017 23:46
jianghaolu pushed a commit to jianghaolu/azure-sdk-for-java that referenced this pull request Feb 27, 2017
[Ready for review] Enable updateResourceAsync
jianghaolu pushed a commit to jianghaolu/azure-sdk-for-java that referenced this pull request Feb 27, 2017
[Ready for review] Enable updateResourceAsync
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.

None yet

5 participants