Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add list of personal repositories that nonowners are pushing to #96

Merged
merged 1 commit into from
Jan 29, 2018

Conversation

larsxschneider
Copy link
Collaborator

Find personal repositories that non-owners are pushing to.
These kind of repositories should be moved into organizations.
Only look at active users (not suspended!) and only look at pushes
of the last 4 weeks.

Copy link
Contributor

@pluehne pluehne left a comment

Choose a reason for hiding this comment

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

Another good metric to keep track of. See my minor comments below 🙂.

@@ -43,6 +43,8 @@
url: "/housekeeping-git-requests"
- title: "API Requests"
url: "/housekeeping-api-requests"
- title: "Personal Non-Owner Push"
Copy link
Contributor

Choose a reason for hiding this comment

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

Merriam-Webster considers “nonowner” a proper word.

Aside from this, could we rename this to “Personal Repositories with Nonowner Pushes”? I think the title is a bit hard to get otherwise.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Any idea for something shorter? Otherwise the menu might look weird

@@ -0,0 +1,4 @@
date personal repositories with non-owner pushes
Copy link
Contributor

Choose a reason for hiding this comment

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

If we decide to rename the report as suggested above, we should change the TSV header as well 🙂.

from .ReportDaily import *

# Find personal repositories that non-owners are pushing to.
# These kind of repositories should be moved into organizations.
Copy link
Contributor

Choose a reason for hiding this comment

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

I’d just write “These repositories should be moved …”

WHERE
users.type = "user"
AND users.suspended_at IS NULL
AND pushes.created_at > DATE_SUB(NOW(), INTERVAL 4 WEEK)
Copy link
Contributor

Choose a reason for hiding this comment

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

As I suggested in my comment on #95, I’d not use NOW() etc.

JOIN users ON repositories.owner_id = users.id
JOIN pushes ON pushes.repository_id = repositories.id
WHERE
users.type = "user"
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, the spacing looks odd to me 🙂.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

aligned to the lines below. not ok?

Copy link
Contributor

@pluehne pluehne left a comment

Choose a reason for hiding this comment

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

Thanks for the adjustments 👍. I have some more minor remarks below.

@@ -43,6 +43,8 @@
url: "/housekeeping-git-requests"
- title: "API Requests"
url: "/housekeeping-api-requests"
- title: "Repository Location"
url: "/housekeeping-repo-personal-non-owner-push"
Copy link
Contributor

Choose a reason for hiding this comment

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

We could shorten the URL as well 🙂.

# of the last 4 weeks.
class ReportReposPersonalNonOwnerPush(ReportDaily):
def name(self):
return "repositories-personal-non-owner-push"
Copy link
Contributor

Choose a reason for hiding this comment

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

Let’s drop the - in non-owner for consistency.

# These repositories should be moved into organizations.
# Only look at active users (not suspended!) and only look at pushes
# of the last 4 weeks.
class ReportReposPersonalNonOwnerPush(ReportDaily):
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to make pushes plural? It’s just a bit odd for me to talk about push in singular (also concerns the output filename repositories-personal-non-owner-push below and the demo data TSV file).

@larsxschneider larsxschneider force-pushed the lars/personal-non-owner branch from 7e8a0e9 to 6f4a2cd Compare January 29, 2018 15:02
<p>
Repositories in user accounts should only be pushed to by their owners as these repositories might become unavailable or deleted if the owner leaves the company.
</p>
<p> Repositories, in which people actively collaborate, should be <a href="https://help.github.com/articles/about-repository-transfers/">transferred</a> to an organization account.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why’s the formatting of this <p> different from the first one?

@larsxschneider larsxschneider force-pushed the lars/personal-non-owner branch from 6f4a2cd to 9f7b4b6 Compare January 29, 2018 15:09
---
layout: default
title: Forks
permalink: /housekeeping-repo-personal-non-owner-push
Copy link
Contributor

Choose a reason for hiding this comment

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

The permalink looks still off?

<div class="chart-placeholder">
<h3>Personal Repositories with Nonowner Pushes in the Last 4 Weeks</h3>
<canvas
data-url="{{ site.dataURL }}/repositories-personal-non-owner-push.tsv"
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you forgot to update the URL 🙂.

Repositories in user accounts should only be pushed to by their owners as these repositories might become unavailable or deleted if the owner leaves the company.
</p>
<p>
Repositories, in which people actively collaborate, should be <a href="https://help.github.com/articles/about-repository-transfers/">transferred</a> to an organization account.
Copy link
Contributor

Choose a reason for hiding this comment

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

The commas in “Repositories, in which people actively collaborate, should …” should not be set, because the “in which” clause is a dependent one (or a concretization of the subject “repositories”).

></canvas>
<div class="info-box">
<p>
Repositories in user accounts should only be pushed to by their owners as these repositories might become unavailable or deleted if the owner leaves the company.
Copy link
Contributor

Choose a reason for hiding this comment

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

I’d put a comma before “… by their owners[,] as these repositories …”

@larsxschneider larsxschneider force-pushed the lars/personal-non-owner branch from 9f7b4b6 to f0dade4 Compare January 29, 2018 15:13
@@ -0,0 +1,38 @@
from .ReportDaily import *

# Find personal repositories that non-owners are pushing to.
Copy link
Contributor

Choose a reason for hiding this comment

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

Might rewrite “non-owners” here as well 🙂.

@larsxschneider larsxschneider force-pushed the lars/personal-non-owner branch from f0dade4 to 7373e6f Compare January 29, 2018 15:16
Find personal repositories that non-owners are pushing to.
These repositories should be moved into organizations.
Only look at active users (not suspended!) and only look at pushes
of the last 4 weeks.
@larsxschneider larsxschneider force-pushed the lars/personal-non-owner branch from 7373e6f to 9715dab Compare January 29, 2018 15:17
Copy link
Contributor

@pluehne pluehne left a comment

Choose a reason for hiding this comment

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

Many thanks for the addressing the suggestions! Everything looks good to me now 😄.

@pluehne pluehne merged commit 9d843e7 into master Jan 29, 2018
@pluehne pluehne deleted the lars/personal-non-owner branch January 29, 2018 15:23
@pluehne pluehne changed the title add list of personal repositories that non-owners are pushing to Add list of personal repositories that nonowners are pushing to Jan 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants