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

restore AclAwareAmazonS3AdapterFactory #284

Merged
merged 3 commits into from
Nov 18, 2024
Merged

Conversation

garak
Copy link
Contributor

@garak garak commented Sep 20, 2024

Fix #283 by restoring the removed class.
If merged, this change should be released in the 0.7 line, e.g. as v0.7.3

@PedroTroller PedroTroller self-assigned this Sep 20, 2024
@garak
Copy link
Contributor Author

garak commented Sep 20, 2024

This doesn't work because I was forced to open the PR against the master branch.
We need a 0.7 branch, created from the latest appropriate tag.

@DanieleScialfa
Copy link

i have the same situation, i wait for update, thanks

@garak
Copy link
Contributor Author

garak commented Nov 13, 2024

@PedroTroller I'm really sorry to bump here, but almost two months passed and I really need this fix. As I suggested before, this PR can be easily reviewed (and hopefully merged) as soon as we have the proper branch.
Thank you.

@alexpozzi
Copy link
Member

@garak I created you a v0.7.x branch matching the latest commit included in the v0.7.2 release.

What you did is fine with me, I let you rebase and, once done, I will review and merge 😉

@garak garak changed the base branch from master to v0.7.x November 14, 2024 10:54
@garak
Copy link
Contributor Author

garak commented Nov 14, 2024

Thank you.
I've adapted the PR to the new branch.
Unfortunately, it seems that the CI has not been triggered. I've tried adding an empty commit, but it hasn't had any effect. I've also tried squashing the two commits, but they haven't had any effect either.
I don't know if you have the possibility to trigger it somehow.

@alexpozzi
Copy link
Member

We can't because v0.7.x was running on Travis CI and that's not working anymore.
Try to rebase again, you should have a CI.

@garak
Copy link
Contributor Author

garak commented Nov 14, 2024

Thanks for the help @alexpozzi but it seems it doesn't work.
I added two commits, and I even renamed the CI directory (it was wrong), but still no luck :-(

@garak
Copy link
Contributor Author

garak commented Nov 14, 2024

My wild guess: https://github.com/KnpLabs/KnpGaufretteBundle/blob/master/.github/workflows/ci.yml#L7 states that only PRs to master trigger the CI.

@alexpozzi
Copy link
Member

I've added the https://github.com/KnpLabs/KnpGaufretteBundle/tree/v0.7.x/.github/workflows/cy.yml (I forgot a trailing s in the workflow folder). I think that now (if you rebase again) it should trigger the ci.

@garak garak force-pushed the restore-class branch 2 times, most recently from 4e6469c to 5411c5e Compare November 14, 2024 15:01
@garak
Copy link
Contributor Author

garak commented Nov 14, 2024

I can't make it work, unfortunately.
Strangely enough, at one point my fork has run it: https://github.com/garak/KnpGaufretteBundle/actions/runs/11839795429

Can it be considered enough? After all, this is a really simple PR, not changing much.

@garak
Copy link
Contributor Author

garak commented Nov 17, 2024

OK, I've found a way to fix all the CI problems on the v0.7.x branch, you can see the passing action here https://github.com/garak/KnpGaufretteBundle/actions/runs/11878238711
As you can see, I also added newer PHP versions to the matrix.

So, even if the action is not applied here, I'm confident everything should be green after the merge.

@alexpozzi alexpozzi merged commit b127fc8 into KnpLabs:v0.7.x Nov 18, 2024
@alexpozzi
Copy link
Member

Thank you!

@garak garak deleted the restore-class branch November 18, 2024 08:12
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.

AclAwareAmazonS3AdapterFactory removed
4 participants