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

ES6: Migrated "assets" #912

Merged
merged 20 commits into from
Mar 22, 2017
Merged

ES6: Migrated "assets" #912

merged 20 commits into from
Mar 22, 2017

Conversation

herrmannplatz
Copy link
Contributor

@herrmannplatz herrmannplatz commented Mar 17, 2017

  • fix test
  • update comments

@herrmannplatz herrmannplatz changed the title 🚧 ES6: migrate "assets" 🚧 ES6: migrate "assets" Mar 17, 2017
@herrmannplatz herrmannplatz changed the title 🚧 ES6: migrate "assets" ES6: Migrated "assets" Mar 17, 2017
return this._build_image_metdadata(file);
} else {
return Promise.resolve();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm no fan of single line if statements.. but applies nicely here:

if (this.is_video(file)) return this._build_video_metdadata(file);
if (this.is_audio(file)) return this._build_audio_metdadata(file);
if (this.is_image(file)) return this._build_image_metdadata(file);
return Promise.resolve();

Copy link
Contributor

Choose a reason for hiding this comment

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

But but but... #consistency

Copy link
Contributor

Choose a reason for hiding this comment

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

like I said.. already.. I'm not a big fan of it..! So I won't mind if not changed :)

return resolve(JSON.parse(this.response));
} else {
return reject(event);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

if (this.status === 201) {
  return resolve(JSON.parse(this.response));
}
return reject(event);


z.assets.AssetUploadFailedReason = {
FAILED: 1,
CANCELLED: 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

alphabetical :)

@herrmannplatz herrmannplatz merged commit 07388e9 into dev Mar 22, 2017
@herrmannplatz herrmannplatz deleted the assets branch March 22, 2017 15:17
@lipis lipis mentioned this pull request Mar 22, 2017
79 tasks
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.

3 participants