Skip to content

Remove reference to non-implemented attributes#26408

Merged
frenck merged 1 commit into
home-assistant:currentfrom
Troon:patch-2
Mar 1, 2023
Merged

Remove reference to non-implemented attributes#26408
frenck merged 1 commit into
home-assistant:currentfrom
Troon:patch-2

Conversation

@Troon
Copy link
Copy Markdown
Contributor

@Troon Troon commented Mar 1, 2023

Scrape integration document refers to the ability to set up attributes, but this is not implemented nor is it planned according to frenck: #26403

I have removed the lines that refer to attributes (not getting confused with the attribute that pulls an attribute from the selected tag — that works and has not been edited).

Proposed change

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: fixes #

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.

Document refers to the ability to set up attributes, but this is not implemented nor is it planned according to frenck:

#26403

I have removed the lines that refer to `attributes` (not getting confused with the `attribute` that pulls an attribute from the selected tag — that works and has not been edited).
@Troon Troon requested review from epenet and fabaff as code owners March 1, 2023 12:50
@home-assistant home-assistant Bot added the current This PR goes into the current branch label Mar 1, 2023
Copy link
Copy Markdown
Contributor

@epenet epenet left a comment

Choose a reason for hiding this comment

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

  • I cannot find any references to attributes in the code
  • sensor attributes are generally frowned upon

So I approve this change 👍

@frenck
Copy link
Copy Markdown
Member

frenck commented Mar 1, 2023

Did it support it before? We did change things around this sensor?

If so, it would mean lost functionality during refactoring, which would mean a bug.

Hence me stating it should be reported in the documentation suggestion linked.

For that reason, I'm not approving this change at this time.

../Frenck

@epenet
Copy link
Copy Markdown
Contributor

epenet commented Mar 1, 2023

Did it support it before? We did change things around this sensor?

If so, it would mean lost functionality during refactoring, which would mean a bug.

Hence me stating it should be reported in the documentation suggestion linked.

For that reason, I'm not approving this change at this time.

../Frenck

I have looked at the code history for homeassistant/components/scrape/sensor.py, and I simply cannot find any reference to attributes as a config option.
I think it was mistakenly added in doc PR #24727

@frenck
Copy link
Copy Markdown
Member

frenck commented Mar 1, 2023

Awesome! Thanks for clearing that up @epenet 👍

../Frenck

@frenck frenck merged commit 300ab3e into home-assistant:current Mar 1, 2023
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 2, 2023
@Troon Troon deleted the patch-2 branch April 7, 2023 10:16
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

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants