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 of git repos for font projects #1262

Merged
merged 35 commits into from
Apr 5, 2017

Conversation

felipesanches
Copy link
Collaborator

This pull request addresses the problems described at issue #1241

Extracting the list into a separate file: fontprojects.py
(issue fonttools#1241)
…s, though.

And some of these are binary only, so they still need to be inspected individually
(issue fonttools#1241)
And annotation dozens of other repos with the issues found in each of them.
(issue fonttools#1241)
so that we can generate things like this summary:
=====================
Number of font repositories in this file:
"TTX": 15
"OK": 41
"UFO": 3
"GH-PAGES": 2
"NOTE": 5
"GFONTS-REPO": 1
"OTF": 12
"?": 35
From the total of 114 repos, 46 are enabled.
Enabled means: "OK" or "NOTE" status.

(issue fonttools#1241)
Number of font repositories in this file:
"TTX": 16
"OK": 47
"UFO": 18
"GH-PAGES": 2
"NOTE": 13
"GFONTS-REPO": 1
"OTF": 16
"404-ERROR": 1
"SOURCE-ONLY": 1
From the total of 115 repos, 60 are enabled.
Enabled means: "OK" or "NOTE" status.

(issue fonttools#1241)
@davelab6
Copy link
Contributor

davelab6 commented Mar 29, 2017

@m4rc1e what do you think?

@m4rc1e m4rc1e self-requested a review March 29, 2017 20:25
@m4rc1e
Copy link
Collaborator

m4rc1e commented Mar 29, 2017

@davelab6 I'll review this. I'm still using our pipeline doc :-) so I need to crosscheck.

For now, a hardcoded python file is ok. We may need to improve this in the future.

@felipesanches
Copy link
Collaborator Author

A git-versioned python list is the most flexible (and comfortable) solution for me. And it can very easily spit out a CSV file, if needed for other purposes.

font-family names, based on the foldernames in the google/fonts repo.
We'll still have to fill the gaps in this table...

Number of font repositories in this file:
"TTX": 18
"OK": 46
"UFO": 17
"GH-PAGES": 2
"NOTE": 13
"GFONTS-REPO": 1
"OTF": 15
"404-ERROR": 1
"?": 802
"SOURCE-ONLY": 1
From the total of 916 repos, 59 are enabled.
Enabled means: "OK" or "NOTE" status.

(issue fonttools#1241)
Now we have 91 repos enabled.

====
Number of font repositories in this file:
"TTX": 15
"OK": 76
"UFO": 18
"GH-PAGES": 2
"NOTE": 15
"GFONTS-REPO": 1
"OTF": 15
"404-ERROR": 1
"?": 777
"SOURCE-ONLY": 2
From the total of 922 repos, 91 are enabled.
Enabled means: "OK" or "NOTE" status.

(issue fonttools#1241)
something is still wrong about the way we schedule and run worker containers and how we decide to give up or try again upon failure.
@m4rc1e
Copy link
Collaborator

m4rc1e commented Apr 4, 2017

I've been improving the G Doc but I now agree with @davelab6. This should be a .csv stored in a top level folder such as ./data

@davelab6
Copy link
Contributor

davelab6 commented Apr 4, 2017 via email

feed the container with data from a GoogleDocs spreadsheet.
But we keep a cache file locally as well just in case (and for versioning purposes)

(issue fonttools#1241)
#!/usr/bin/env python
import csv
import urllib
CACHE_CSV = "fontprojects.csv"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not store this in memory using StringIO?

https://docs.python.org/2/library/stringio.html

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do store the data on disk on purpose. So that I can keep track of history on the git repo. I'm OK with fetching data from a remote spreadsheet, but I also like the guarantee of having access to the data even if the spreadsheet goes offline.

Copy link
Collaborator

@m4rc1e m4rc1e left a comment

Choose a reason for hiding this comment

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

Overall, a fairly elegant way to get the Google Doc into a .csv, without using gspread. It made me smile.

It would be nice to move this out of the dashboards directory. This functionality is not just related to the dashboard.

@m4rc1e
Copy link
Collaborator

m4rc1e commented Apr 4, 2017

@davelab6 Felipe's solution uses the Google Doc without the need for fancy dependencies which require authentication. He's cracked it.

CACHE_CSV = "fontprojects.csv"

def get_repos():
handle = open(CACHE_CSV)
Copy link
Collaborator

@m4rc1e m4rc1e Apr 4, 2017

Choose a reason for hiding this comment

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

You're not closing the file in this function.

I'd opt for this:

def get_repos():
  with open(CACHE_CSV) as handle:
      repos = []
      for row in csv.reader(handle):
        if not row:
          continue
        repos.append(row)
      return repos

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If I'm not mistaken, garbage collection does its work of closing the file upon returning the function call, as there's no remaining reference to the file object.

@felipesanches
Copy link
Collaborator Author

It would be nice to move this out of the dashboards directory. This functionality is not just related to the dashboard.

There are some limitations imposed by the Dockerfile, which does not allow references to parent folders. So we have to keep the code in the container folder. The only way out of this that I can see would be to split out the dashboard as a separate git repo. There are other considerations to make if we go that way, though.

@felipesanches felipesanches merged commit 94cde5c into fonttools:master Apr 5, 2017
@felipesanches
Copy link
Collaborator Author

I have merged this because it contains fixes to actual fontbakery checks. Off course, expending is an ongoing effort, so we'll probably open more PRs for that later.

@m4rc1e
Copy link
Collaborator

m4rc1e commented Apr 5, 2017

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants