-
Notifications
You must be signed in to change notification settings - Fork 72
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
Switch to YAML and GitHub data (WIP) #1063
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking forward to this work being ready.
Are you planning to add a workflow for this?
gh-data.py
Outdated
for label in labels: | ||
# Position | ||
if label["name"] == "blocked": | ||
assert position is None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might pay to turn these assertions into warning messages instead. We can have a -Werror
analogue so that we can ensure that they surface, but the idea that this whole process aborts if we mess up the labeling worries me.
position = "blocked" | ||
elif label["name"].startswith("position: "): | ||
assert position is None | ||
position = label["name"][len("position: ") :] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
position = label["name"][len("position: ") :] | |
position = label["name"].removeprefix("position: ") |
&c...
Though maybe label["name"].split(": ")
is a better implement.
timeout=5, | ||
) | ||
response.raise_for_status() | ||
except Exception: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might pay to check for rate limits. We have a fair number of issues and I have first-hand experience with hitting those limits. Look at this code for some clues on how to manage that.
Also, this request is not using $GITHUB_TOKEN
, but I've found that doing that helps a fair bit. It means setting a header, but it means that you are less likely to hit rate limits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28#primary-rate-limit-for-unauthenticated-users says 60 requests per hour, and currently this will make 12 requests. (Could make it avoid the last request by looking at the link
response header.)
If we make a GH Action workflow to batch updates if the most recent run was less than 15 minutes ago or so (which seems like a good idea anyway), unauthenticated shouldn't hit the rate limit for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've experimented in https://github.com/zcorpan/gh-actions-test to get such a setup working (with help of ChatGPT 4o). The workflow will push built files to gh-pages
, which I think is useful to be able to see the history.
@@ -0,0 +1,2798 @@ | |||
<link>'s imagesrcset and imagesizes attributes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm constantly amazed at YAML and its ability to interpret what would be garbage input in other contexts. I assume that this is machine-generated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is yes (json2yaml.py
)
@@ -0,0 +1,9731 @@ | |||
[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this file need to be checked in to the repository?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, at least not to main
. My plan is to check in generated files to gh-pages
with a GH Action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed with this direction and simplification.
No description provided.