Skip to content

Add landingpage for removed integrations#16120

Merged
frenck merged 7 commits into
currentfrom
adr0004
Jan 11, 2021
Merged

Add landingpage for removed integrations#16120
frenck merged 7 commits into
currentfrom
adr0004

Conversation

@ludeeus
Copy link
Copy Markdown
Member

@ludeeus ludeeus commented Jan 7, 2021

Proposed change

I'm not 100% sure about this one.
On one hand having something, other than the image below is IMO better than having something that does not look like you still are on our site anymore. On the other hand, this adds another list that will need to be maintained for every integration that is being removed, and once it's out why should we care about old links.

image

Feel free to close the PR if this is not wanted 👍

Type of change

  • Spelling, grammar or other readability improvements (current branch).
  • Adjusted missing or incorrect information in the current documentation (current branch).
  • Added documentation for a new integration I'm adding to Home Assistant (next branch).
  • Added documentation for a new feature I'm adding to Home Assistant (next branch).
  • Removed stale or deprecated documentation.

Additional information

  • Link to parent pull request in the codebase:
  • Link to parent pull request in the Brands repository:
  • This PR fixes or closes issue:

Checklist

  • This PR uses the correct branch, based on one of the following:
    • I made a change to the existing documentation and used the current branch.
    • I made a change that is related to an upcoming version of Home Assistant and used the next branch.
  • The documentation follows the Home Assistant documentation standards.

@probot-home-assistant probot-home-assistant Bot added the current This PR goes into the current branch label Jan 7, 2021
Comment thread source/more-info/adr0004.markdown Outdated
Comment thread source/more-info/adr0004.markdown Outdated
ludeeus and others added 2 commits January 8, 2021 10:56
Co-authored-by: Hmmbob <33529490+hmmbob@users.noreply.github.com>
Co-authored-by: Hmmbob <33529490+hmmbob@users.noreply.github.com>
@frenck
Copy link
Copy Markdown
Member

frenck commented Jan 8, 2021

The problem with this approach is that this is throwing a 200 status in the end, thus the result is not removed from Google.

@ludeeus
Copy link
Copy Markdown
Member Author

ludeeus commented Jan 8, 2021

For that, we can add 301 to the redirect rule

Comment thread source/more-info/adr0004.markdown Outdated
Comment thread source/more-info/adr0004.markdown Outdated
Comment thread source/more-info/adr0004.markdown Outdated
@frenck
Copy link
Copy Markdown
Member

frenck commented Jan 8, 2021

For that, we can add 301 to the redirect rule

That is probably a better solution, yes.

@frenck
Copy link
Copy Markdown
Member

frenck commented Jan 8, 2021

Just thinking a bit, do we need to tell why it was removed?

As in, we remove integrations for more reasons. The point is, handling the 404 of an integration that was once there. Maybe just a generic page with: "Yo, there was an integration here once, which has been removed from Home Assistant. Possible causes: - unmaintained / no longer working with devices / violating design decisions."-ish page.

@hmmbob
Copy link
Copy Markdown
Contributor

hmmbob commented Jan 8, 2021

For that, we can add 301 to the redirect rule

Why not 404? Seems possible to redirect to an error page and throw a 404 - if I look at the docs, no personal experience though

https://docs.netlify.com/routing/redirects/redirect-options/

@ludeeus
Copy link
Copy Markdown
Member Author

ludeeus commented Jan 11, 2021

Just thinking a bit, do we need to tell why it was removed?

Good point!
I'll restructure with 301 and make it for generic integration removal

@ludeeus ludeeus changed the title Add landingpage for integration removed ofr ADR0004 Add landingpage for removed integrations Jan 11, 2021
Comment thread source/more-info/removed-integration.markdown Outdated
Copy link
Copy Markdown
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

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

Awesome! 👍

@frenck frenck merged commit 406d8d3 into current Jan 11, 2021
@frenck frenck deleted the adr0004 branch January 11, 2021 10:49
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

current This PR goes into the current branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants