Skip to content

feat: [#91] added image proxy with cache#134

Merged
mickvandijke merged 3 commits into
torrust:developfrom
mickvandijke:rebase-91-add-endpoint-for-image-proxying
May 4, 2023
Merged

feat: [#91] added image proxy with cache#134
mickvandijke merged 3 commits into
torrust:developfrom
mickvandijke:rebase-91-add-endpoint-for-image-proxying

Conversation

@mickvandijke

@mickvandijke mickvandijke commented May 3, 2023

Copy link
Copy Markdown
Collaborator

Rebased and processed @josecelano 's feedback: #100

@mickvandijke mickvandijke requested review from da2ce7 and josecelano May 3, 2023 14:28
josecelano
josecelano approved these changes May 3, 2023
View reviewed changes

@josecelano josecelano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey @WarmBeer, it looks good to me.

Comment thread Cargo.toml Outdated
Comment on lines +44 to +46
bytes = "1.4.0"
text-to-png = "0.2.0"
indexmap = "1.9.3"

@josecelano josecelano May 3, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @WarmBeer, I changed all dependencies to use only the minor versions so that you can upgrade patches.

bytes = "1.4"
text-to-png = "0.2"
indexmap = "1.9"

@josecelano josecelano added the Needs Rebase Base Branch has Incompatibilities label May 3, 2023
@mickvandijke mickvandijke force-pushed the rebase-91-add-endpoint-for-image-proxying branch 2 times, most recently from 39a6218 to dc279db Compare May 4, 2023 10:44
@mickvandijke mickvandijke force-pushed the rebase-91-add-endpoint-for-image-proxying branch from dc279db to 916d869 Compare May 4, 2023 11:04
@mickvandijke

mickvandijke commented May 4, 2023

Copy link
Copy Markdown
Collaborator Author

Hi @josecelano ,

I still have 1 failing test after rebasing. Is there something else I need to do?

@josecelano

josecelano commented May 4, 2023

Copy link
Copy Markdown
Member

Hi @josecelano ,

I still have 1 failing test after rebasing. Is there something else I need to do?

@WarmBeer, that's the test I knew would fail because you added a new section to the configuration. Let's merge the PR, and I will fix it since I will start working on this issue, which is related to configuration. But just for let you know, you only need to add the new section in here.

@josecelano

josecelano commented May 4, 2023

Copy link
Copy Markdown
Member

Hi @josecelano ,
I still have 1 failing test after rebasing. Is there something else I need to do?

@WarmBeer, that's the test I knew would fail because you added a new section to the configuration. Let's merge the PR, and I will fix it since I will start working on this issue, which is related to configuration. But just for let you know, you only need to add the new section in here.

I'm fixing it in a new branch with a new commit. I will replace this PR with the new one in a few minutes.

@mickvandijke mickvandijke merged commit 7cd0cb2 into torrust:develop May 4, 2023
@mickvandijke mickvandijke removed the Needs Rebase Base Branch has Incompatibilities label May 4, 2023
@josecelano josecelano mentioned this pull request May 4, 2023
Closed
@josecelano

josecelano commented May 4, 2023

Copy link
Copy Markdown
Member

@WarmBeer this is the new PR fixing the test.

@josecelano josecelano mentioned this pull request May 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@josecelano josecelano josecelano approved these changes

@da2ce7 da2ce7 Awaiting requested review from da2ce7

Assignees

No one assigned

Labels

None yet

Projects

Torrust Solution
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mickvandijke @josecelano