-
Notifications
You must be signed in to change notification settings - Fork 29
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
adding more blinka repos #192
adding more blinka repos #192
Conversation
I think it will be more involved than this. For instance this line has the repo hardcoded: |
Good catch, thank you! The latest commit is an attempt to fix that, but is still untested. I can try to spend some time this week working my way through the logic in this script to see if there are more changes that will be needed. Maybe there is some way I can tweak it some to use fewer API calls to try to narrow down my testing to just the portion I am actively changing. |
I think I remember hearing something about an adabot console, which may be the correct way to test this, but don't have much information on that. |
It may be an issue of authenticated API calls vs. unauthenticated ones as well. It seems the rate limit is much higher for authenticated ones so perhaps it's just a matter of getting it to make the requests from my user instead of unauthenticated. |
I think we might be good to go with the latest version. I was able to successfully run the report by providing my username and token in an environment variable and it did increase my API rate limit enough that I can have about 3 tries per hour if it completes the full thing. Here is the Blinka section that it generated:
at the beginning it printed this:
which does show 6 additional repos as compared to the output from actions in the most recent report run: https://github.com/adafruit/adabot/runs/1345816601?check_suite_focus=true#step:9:14 For reference here is the output from the report without the changes in this PR:
|
This looks good. Maybe make it so that 2 repos lists don't need to be updated by doing something like:
|
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.
Looks great.
Thanks @makermelissa, its much nicer this way without the duplicated list. I didn't know it was so easy to concatenate tuples together like that. |
Yeah, it's pretty easy with Python. This was a little trickier since we're concatenating a list and tuple, hence the tuple wrapper. Also, we could have made the blinka list a tuple instead of a list in order to simplify things, but I wasn't worried about adabot needing to be super memory efficient. |
I think this may be what is needed to resolve #189.
I tried testing it out locally by running
circuitpython_libraries.py
I found that I had to insert my github username somewhere to get it working. And once I did that it seemed to start running, but then reported hitting my github API rate limit and started sleeping for ~1hr before presumably continuing.I'm not certain what the rate limit is set to, nor how many calls this script is attempting to make, so I'm not sure if it's actually practical for me to be able to test it or not.
Either way I am happy to add or change anything else that needs to get updated for this if someone can point me in the right direction.