Filestack Transformations Support

[scudco]

@mminkoff take a look at the updated README.md

Here's what I've still got to do before I feel like this is mergeable:

• Move tests into more accurate locations
  • Currently, everything is being tested via the handlebars helper
• Add more information on what partial support of the resize transformation means
  • Consider adding "full support" for a transformation so we know what that could look like
    • Will be addressed in a future PR
• Add example of how to add first-class support for a transformation
  • Where does the code go? What does it take in? What does it spit out?
• Figure out how non-image transformations will work
  • These are technically possible given the current API, but a future PR will add explicit support.
• Figure out if it's a good idea to extract the main content of imageUrl into a standalone class/object.
  • This may be in a future PR.

[mminkoff]

I just made some minor edits to the readme - I hope you don't mind.

This looks great! Perhaps add a note about welcoming pull requests with additional transformations? I wouldn't want to hold up releasing this because not all the transformations are complete.

This is a terrific step forward - can't wait to get it out!

[scudco]

okay @mminkoff I think I feel good about this now that I've added service-specific tests and some more information in the README like you suggested. Please feel free to edit any of this as appropriate.

[mminkoff]

This is all so great - I love what you've done.

I have one last concern about the README. I'm just a little confused by the transformations and I think we should clarify. If I understand correctly, the default is that it will accept any transformation and attempt to assemble the proper query string (which has been the case all along). However, there are some (resize for now) that have additional intelligence to help ensure that you don't make a mistake. Correct?

The way it reads now (to me) it seems like most of the transformations have no support and resize has limited support. Know what I mean? Can you clarify that? Or confirm that I understand it correctly and I'll give it a shot.

[scudco]

I'm not really happy about having to include ember-assign-polyfill as it's going to emit warnings for most users, but it gets the build working.

To remove the polyfill I think we'd have to do one of the following:

• change to a different method of doing what is assign is doing
• drop support for Ember <2.5
• conditionally include the polyfill somehow

What do you think @mminkoff?

[scudco]

@mminkoff yeah I see what you mean. I think the problem is with the word 'support'. I will try to think of a way to rephrase to make it less confusing.

Do you think the table with all the empty cells makes it worse? I wanted to list everything so that we could start working down that list to add first-class support, but maybe seeing a bunch of empty cells could make someone think they can't use it?

[mminkoff]

@scudco Yes, I do think the blank cells makes it worse, though I think having the table with the links is great. Perhaps there's a "weaker" checkbox icon we could us for standard support, rather than just leaving it blank? Also it'd be good to define what "full support" and "partial support" means.

[mminkoff]

@scudco how/where are we using assign?

[mminkoff]

@scudco I suppose it's better to simplify the legend like that, though there's something wrong - it's not completely done it seems. Perhaps it could have a note at the top that some transformations have additional support code that helps ensure that parameters are used properly?

[scudco]

Yeah consider this still a WIP. I had to get to something else and wanted to commit what I had done. I need a way to point out that transformations may behave differently if noted without sounding really weird.

[scudco]

I added a couple sentences above table. I still think it sounds awkward, but at least that lone note next to resize isn't totally out of nowhere.

[mminkoff]

I tweaked it just a bit more and am comfortable (not to mention very excited) pushing this if you are @scudco.

[scudco]

yeah that looks great @mminkoff. thanks for adding clearer language :)

⛵️