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

Factor error and success methods from services. #7807

Merged
merged 1 commit into from Sep 22, 2014

Conversation

4 participants
@cirosantilli
Contributor

cirosantilli commented Sep 21, 2014

The most important factorization here is to fix a single status: :error and status: :sucess keys for all services.

The DeleteBranch service had already started diverging as is used the key :state instead of :status.

For the error, it was also possible to factor out the message as it is used on all returns. The same was not possible for success.

The initialize signature was modified in order to allow to use inheritance. This also makes the execute function parameters lists shorter since project and current_user now go on the initializer like all other services.

The next step would be to have an uniform error reporting across all services: some create services currently return the created object with errors added by .errors.add. Maybe we should just follow that for all services as it is cleaner than playing with hashes.

@TeatroIO

This comment has been minimized.

TeatroIO commented Sep 21, 2014

I've prepared a stage. Click to open.

@cirosantilli cirosantilli force-pushed the cirosantilli:factor-service-error branch 2 times, most recently from dec4a8c to d8cf9ef Sep 21, 2014

@cirosantilli cirosantilli force-pushed the cirosantilli:factor-service-error branch from d8cf9ef to ad47993 Sep 21, 2014

@Razer6

This comment has been minimized.

Member

Razer6 commented Sep 21, 2014

👍

@Razer6 Razer6 added this to the 7.4 milestone Sep 21, 2014

dzaporozhets added a commit that referenced this pull request Sep 22, 2014

Merge pull request #7807 from cirosantilli/factor-service-error
Factor error and success methods from services.

@dzaporozhets dzaporozhets merged commit db12e2d into gitlabhq:master Sep 22, 2014

1 check passed

default The build passed on Semaphore.
Details

@cirosantilli cirosantilli deleted the cirosantilli:factor-service-error branch Sep 22, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment