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

Update S3 uploads dependency #333

Closed
tareiking opened this issue Mar 3, 2021 · 10 comments
Closed

Update S3 uploads dependency #333

tareiking opened this issue Mar 3, 2021 · 10 comments
Labels
task An item with a singular purpose and clearly defined path forward
Milestone

Comments

@tareiking
Copy link

tareiking commented Mar 3, 2021

Hey, we could really use the humanmade/S3-Uploads#493 update in S3 uploads that @goldenapples worked on for other projects.

We are using Altis V4 and moving to V5 shortly, so two things would need to happen from:

  • S3 uploads repo gets new tag and version bump
  • Altis Cloud gets a new tag and version bump

It would be ideal to get a timeline on this so we can set expectations with the client.

Is there an alternative path for updating our project altis-cloud dependency on s3 manually that would work ideally here for future upgrade paths?

Edit: Or can we backport cherry-picked features if pinning a new version is not in line with delivery/roadmap.

@roborourke
Copy link
Contributor

S3 Uploads is tracked by the cloud module at any 2.x version so we just need to do a release of the S3 Uploads plugin itself.

@roborourke
Copy link
Contributor

Seems the master branch upstream has breaking changes so would need to make a v2-branch and add backport bot for that repo to make a release more quickly.

@tareiking there are ways to trick composer using aliases eg. "humanmade/s3-uploads": "dev-master as 2.99.0" but we can't really offer support if you went that route. You'll hit all kinds of potential issues as we couldn't guarantee your version of Altis is compatible.

@roborourke roborourke added the task An item with a singular purpose and clearly defined path forward label Mar 9, 2021
@roborourke
Copy link
Contributor

Looking into this further Altis is not currently compatible with the latest changes in S3 Uploads so it's a little more complicated to update this.

@tareiking I'm afraid the current changes in S3 Uploads beyond the v2-branch cannot be backported to earlier versions.

We do still need to upgrade the plugin however but also ensure Altis supports it.

Steps required are:

  • Release v3.0.0 of S3 Uploads
  • Update plugin version constraint in this repo
  • Fix any references to non-namespaced S3 Uploads classes / functions across the whole of Altis

@goldenapples
Copy link
Contributor

I looked into this in humanmade/S3-Uploads#401 (comment) as well. The last commit in S3-Uploads before the namespace reorganization was 539d0c1, and the patch from humanmade/S3-Uploads#493 should apply cleanly at that point. The blocking question was that this introduced substantial new functionality to the v2 branch that really should be a major version bump. But maybe it makes sense to cut this as a 2.3-beta release, so that projects can choose to opt into it rather than tracking feature branches with cherry picked commits.

@roborourke
Copy link
Contributor

If you can create a PR against the v2-branch that would be appreciated, then we could do a 2.3 release. Assuming there are no breaking changes there. I just wasn't able to use the automated backport tool and I don't have time to do this myself.

@joehoyle would appreciate your thoughts regarding this change as you did the review. I'm not sure how wide reaching it is or what the risk might be.

@joehoyle
Copy link
Member

I'm fine backporting https://github.com/humanmade/S3-Uploads/pull/493/files, however will probably just need a copy/paste as I don't think the patch willy apply very easily. I don't want to put all the namespace reorganization into a 2.3-beta though (not totally sure if that was suggested), so I think it should still be the case that if you want, say, private uploads, you need to move to 3.0.0.

@roborourke
Copy link
Contributor

Yeah, this should only be for the performance improvement piece itself.

Altis v7 will have S3 Uploads v3, then we can look at 1st party support for private uploads in v8 perhaps.

@goldenapples
Copy link
Contributor

Cool, I opened humanmade/S3-Uploads#497 from manually cherry-picking the commits from the branch I was working from.

@tareiking just to be aware, this will only add the required attachment meta for newly uploaded attachments. If you want to fix old attachments, running wp media regenerate after this patch is in will update them.

@tareiking
Copy link
Author

Thanks all. That makes sense about the versioning and back porting.

@tareiking just to be aware, this will only add the required attachment meta for newly uploaded attachments. If you want to fix old attachments, running wp media regenerate after this patch is in will update them.

Ah @goldenapples - your past self is awesome, I was aware of this as you'd previously commented in your original PR h/t.

@roborourke
Copy link
Contributor

So @joehoyle has tagged 2.2.3 of S3 Uploads with @goldenapples' fix.

This means you can run composer update --with-all-dependencies to update S3 Uploads as we track 2.2.x in this module.

The master branch is tracking 3.0.0-alpha which also has these changes.

Thanks both for your work here!

@roborourke roborourke added this to the v7 sprint 4 milestone Mar 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
task An item with a singular purpose and clearly defined path forward
Projects
None yet
Development

No branches or pull requests

4 participants