Skip to content

ci: update redirects with replace directive in s3 bucket - #15109

Merged
usha-mandya merged 1 commit into
docker:masterfrom
crazy-max:redirects
Jul 13, 2022
Merged

ci: update redirects with replace directive in s3 bucket#15109
usha-mandya merged 1 commit into
docker:masterfrom
crazy-max:redirects

Conversation

@crazy-max

@crazy-maxcrazy-max commented Jul 13, 2022•
edited
Loading

Copy link
Copy Markdown
Member

Handles redirects generated by https://docs.docker.com/redirects.json in our s3 bucket by setting redirect requests for the objects. As our bucket is configured as a website, objects with redirects should have the proper 301 redirect status.

Signed-off-by: CrazyMax crazy-max@users.noreply.github.com

@crazy-max
crazy-max requested a review from thaJeztahJuly 13, 2022 11:52
@netlify

netlifyBot commented Jul 13, 2022•
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for docsdocker ready!

Built without sensitive environment variables

NameLink
🔨 Latest commitba3ca84
🔍 Latest deploy loghttps://app.netlify.com/sites/docsdocker/deploys/62ced21059875f0009a92a51
😎 Deploy Previewhttps://deploy-preview-15109--docsdocker.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@crazy-max

crazy-max commented Jul 13, 2022

Copy link
Copy Markdown
MemberAuthor

@thaJeztah Is it https://github.com/docker/docker.github.io/blob/master/_data/redirects.csv still used?

@thaJeztah

thaJeztah commented Jul 13, 2022

Copy link
Copy Markdown
Member

Goooooood question. TBH I forgot that was there; looks like that was part of e3cb726, but I'd have to dig as well to see how (and IF) it's still used.

@thaJeztah

thaJeztah commented Jul 13, 2022

Copy link
Copy Markdown
Member

A quick search tells me it's no longer used 🤔 but please double check if you find some reference to it

@thaJeztah

thaJeztah commented Jul 13, 2022

Copy link
Copy Markdown
Member

Oh! Perhaps I left it as a reminder to "include these as redirect_from"

thaJeztah
thaJeztah reviewed Jul 13, 2022
View reviewed changes
Comment thread.github/workflows/publish.yml Outdated
thaJeztah
thaJeztah requested changes Jul 13, 2022
View reviewed changes
Comment thread.github/workflows/publish.yml Outdated
Comment thread.github/workflows/publish.yml Outdated
Comment thread.github/workflows/publish.yml Outdated
Comment thread.github/workflows/publish.yml Outdated
@crazy-max
ci: update redirects with replace directive in s3 bucket
ba3ca84
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
thaJeztah
thaJeztah approved these changes Jul 13, 2022
View reviewed changes

@thaJeztahthaJeztah 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.

LGTM

I guess the only way to try this one is to merge and check docs-stage.docker.com ?

@crazy-max

crazy-max commented Jul 13, 2022

Copy link
Copy Markdown
MemberAuthor

LGTM

I guess the only way to try this one is to merge and check docs-stage.docker.com ?

Yes indeed

@usha-mandya
usha-mandya merged commit 85c5ba5 into docker:masterJul 13, 2022
@crazy-maxcrazy-max mentioned this pull request Jul 13, 2022
Merged
crazy-max added a commit to crazy-max/docker-docs that referenced this pull request Jul 13, 2022
@crazy-max
Revert "Merge pull requestdocker#15109from crazy-max/redirects"
387a050
This reverts commit 85c5ba5, reversing
changes made to 23b73f2.
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max
crazy-max deleted the redirects branch July 15, 2022 19:17
This was referenced Jul 15, 2022
Merged
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztahthaJeztahthaJeztah approved these changes
@StefanSchererStefanSchererAwaiting requested review from StefanScherer

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@crazy-max@thaJeztah@usha-mandya