smeevil / cloudex

An elixir library which helps with uploading image files or urls to cloudinary
Do What The F*ck You Want To Public License
103 stars 64 forks source link

Cloudex.upload returns list instead of map #32

Closed billc closed 6 years ago

billc commented 6 years ago

From the documentation I was expecting Cloudex.upload to return a traditional {:ok, %Cloudex.UploadedImage{}}. However, the code is returning a list as [ok: %Cloudex.UploadImage{}].

Is this expected? I can understand returning a list if a list was passed as input to the upload method, but not in that form.

smeevil commented 6 years ago

Hi, I agree that this is an anomaly indeed. Ideally it should return a tuple if only one file is uploaded and a list of tuples when multiple where uploaded.

I'd like to change this behaviour but do not really know what the best way would be to make this change with the least amount of disruption for the current users.

One possibility would be to do a major version increase to make this change, with a clear note of the breaking change, and the usual annotation in the changelog. I'm also open for more suggestions on how to properly make this change. Pinging some contributors to see if they might have some input as well on this :) @nathanl @Tyler-pierce @pauloancheta @nbancajas @sudostack @remiq

Thanks in advance, Gerard

nathanl commented 6 years ago

I'd vote for "make the change and do a major version bump, explaining in the changelog." I see that Cloudex isn't at 1.0 yet, but I think it could be. Nothing is ever complete, but what it does, it does well.

billc commented 6 years ago

+1 major version change to notify existing users of the breaking interface. Curious, as I am the first to mention the discrepancy, are most users passing a list. Wonder what percentage of users are passing a single, non-list input to the method.

pauloancheta commented 6 years ago

+1 on major change and explain in the changelog. And as @billc have mentioned, there might not be a lot of users a non-list input to the method. Disruption to other users would be minimal.

smeevil commented 6 years ago

Done :) 1.0.0 is now published, returning {:ok, %Cloudex.UploadedImage{}} for a single file / url and a list of those when uploading multiple.