Skip to content

Conversation

@renemarc
Copy link
Contributor

@renemarc renemarc commented Mar 27, 2020

Breaking change

Proposed change

As a developer, it always feels good to see your code used and enjoyed by many people. To measure such code usefulness to the world, GitHub showcases a few key indicators for each repos: number of watchers, stargazers, contributors, forks, clones, views, issues, PRs.

The GitHub integration already offers a few:

  • Stargazers
  • Open Issues
  • Open PRs

This PR aims to add these other health counters:

  • Forks
  • Clones (all, uniques)
  • Views (all, uniques)

While more data means more API calls, the usefulness of the returned indicators justifies the few extra queries.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

@bdraco
Copy link
Member

bdraco commented Apr 5, 2020

@timmo001 Can you take a look at this?

Copy link
Member

@timmo001 timmo001 left a comment

Choose a reason for hiding this comment

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

Looks good. New attributes pulled correctly.

Screenshot_20200405_234048

Only suggestion would be to use unique instead, however this is what the GH API uses so isn't a big deal

@bdraco
Copy link
Member

bdraco commented Apr 5, 2020

Thanks @timmo001

@bdraco bdraco self-assigned this Apr 5, 2020
@bdraco
Copy link
Member

bdraco commented Apr 7, 2020

@renemarc Do you want to make adjustments or merge as-is?

@renemarc
Copy link
Contributor Author

renemarc commented Apr 8, 2020

@bdraco Sure, I'll make the suggested changes and push a commit shortly!

And thanks @timmo001 for the code review!

@renemarc
Copy link
Contributor Author

renemarc commented Apr 8, 2020

@timmo001 The PR was updated with your suggestions!

@Mariusthvdb
Copy link
Contributor

Mariusthvdb commented Apr 8, 2020

hi,
very interested in this since I have made a full package following my GitHub repos for the custom cards, and have automated their update availability etc. So, really nice you added these attributes to the sensor. thanks!

What I would really appreciate if you could somehow add the date of the latest update to the attributes. Would that be possible?

using last_changed now is useless, since it doesn't restore over HA restarts. We really need the true date of the last update on the Github repo itself.
Schermafbeelding 2020-04-08 om 09 47 30

Schermafbeelding 2020-04-08 om 09 47 15

thanks for considering

@bdraco
Copy link
Member

bdraco commented Apr 8, 2020

using last_changed now is useless, since it doesn't restore over HA restarts. We really need the true date of the last update on the Github repo itself.
Schermafbeelding 2020-04-08 om 09 47 30

If you make it a RestoreEntity(see https://community.home-assistant.io/t/main-purpose-of-restoreentity/135270/4) you can restore the start on restart.

Let's keep the scope as-is on this one. If someone wants to do that in a followup it will be much easier to review small chunks

@bdraco bdraco merged commit f5b7ded into home-assistant:dev Apr 8, 2020
@renemarc renemarc deleted the feat/github-forks-clones-views branch April 10, 2020 04:04
@lock lock bot locked and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants