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

List: organization owners #36

Closed
wants to merge 9 commits into from
Closed

List: organization owners #36

wants to merge 9 commits into from

Conversation

mlbright
Copy link
Contributor

@mlbright mlbright commented Nov 8, 2017

    * This was Patrick's reverse engineering efforts
    * We may have a ghe-console script from GitHub support
    (https://support.enterprise.github.com/hc/en-us/requests/51994)
    * We need to remove service accounts
* scaffolding for showing the above results
@toddocon
Copy link
Contributor

toddocon commented Nov 8, 2017

I recreated your files in my (older Hubble) dev env and ran into a file generation problem. The error was:

self.executeScript(self.scriptPath("org-owners.sh"))
AttributeError: 'ReportOrgOwners' object has no attribute 'scriptPath'

So, in ReportOrgOwners.py, I changed this

self.executeScript(self.scriptPath("org-owners.sh"))

to this

self.executeScript(os.path.join("scripts", "org-owners.sh"))

Now the data file is built. My output, though, is a table 10,221 rows long. There is one owner per line and took about 24 seconds to build.

@mlbright
Copy link
Contributor Author

mlbright commented Nov 9, 2017

@toddocon I fixed the ghe-console script (thank you GHE support!)

@mlbright
Copy link
Contributor Author

mlbright commented Nov 9, 2017

I'm not sure why you need to specify 'scripts' and I don't.

@pluehne pluehne self-assigned this Nov 13, 2017
@pluehne pluehne changed the title organization owners List: organization owners Nov 13, 2017
@pluehne
Copy link
Contributor

pluehne commented Nov 13, 2017

@toddocon: It’s weird that

self.executeScript(self.scriptPath("org-owners.sh"))

didn’t work for you, but

self.executeScript(os.path.join("scripts", "org-owners.sh"))

did. In fact, self.scriptPath should eventually do the same thing.

Also, other reports (such as for Git versions) use the same call. Are they broken for you as well?

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.

@mlbright: Your changes look good to me 😃. I listed some tiny details below.

We’ll also need some demo data for the interactive web demo.

And, similar to what @toddocon mentioned, it would be better to have all the owners of each organization on the same output line.

I can help with all that, just give me write access to your branch 😃.

permalink: /org-owners
---
<h3>
Organization owners
Copy link
Contributor

Choose a reason for hiding this comment

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

@mlbright: Can you make this title case (“Organization Owners”)?

@@ -21,6 +21,7 @@
from reports.ReportRepositoryHistory import *
from reports.ReportTokenlessAuth import *
from reports.ReportUsers import *
from reports.ReportOrgOwners import *
Copy link
Contributor

Choose a reason for hiding this comment

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

@mlbright: Could you please sort these alphabetically?

@@ -82,6 +83,7 @@ def main():
ReportRepositoryHistory(configuration, dataDirectory, metaStats).update()
ReportTokenlessAuth(configuration, dataDirectory, metaStats).update()
ReportUsers(configuration, dataDirectory, metaStats).update()
ReportOrgOwners(configuration, dataDirectory, metaStats).update()
Copy link
Contributor

Choose a reason for hiding this comment

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

@mlbright: Could you please sort these alphabetically?

@pluehne
Copy link
Contributor

pluehne commented Nov 13, 2017

For future versions of this report, I’d like the following improvements:

  • a search box to quickly find the owners of a specific organization (which, I think, is the typical use case of this report)
  • links to the user accounts of the owners, just like the organizations themselves are linked

But for now, I think this report is already very useful 😃.

<div class="row">
<div class="col-main">
<div class="chart-container">
<table class="table" data-url="{{ site.dataURL }}/org-owners.tsv"></table>
Copy link
Contributor

Choose a reason for hiding this comment

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

@mlbright: You accidentally mixed spaces and tabs here.

</div>
<div class="col-main">
<div class="chart-container">
<table class="table" data-url="{{ site.dataURL }}/org-owners.tsv"></table>
Copy link
Contributor

@pluehne pluehne Nov 14, 2017

Choose a reason for hiding this comment

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

Sorry, I should have been more specific. We’re using tabs in the other report files, and it would be best to keep that consistent.

@pluehne
Copy link
Contributor

pluehne commented Nov 15, 2017

As I don’t have write access to @mlbright’s branch, I branched this pull request.

7e9ce8d adds some demo data for autodesk.github.io, and be7a159 is a work-in-progress commit, where I started to support multiple owner names on a single line in the table (with links to their GitHub profiles). I left this work-in-progress, as I’ll be out of office until next week.

Note that the updater report’s output format needs to be changed in the same way (instead of having one line for each (organization, owner) pair, just output a single line for each organization with all the owners comma-separated). Also, the column heading should be owners and not owner (singular).

@larsxschneider
Copy link
Collaborator

Superseded by #38

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.

4 participants