Skip to content

Conversation

@GeoffRiley
Copy link
Contributor

Difficulty level (1-10): [7]
Estimated time spent (hours): [20]
Completed (yes/no): [Yes]
I stretched my coding skills (if yes what did you learn?): [Talking to the GitHub API, using the bottle mini web server… avoiding overloading the GitHub API limit :)]
Other feedback (what can we improve?): [It's grand… it just took me from last Hacktoberfest to this one to get it to work properly.]

Kept exceeding API limits at GitHub.  Need a better understanding of the API before trying again.
All sorted now, bottle app working nicely.
Copy link
Collaborator

@bbelderbos bbelderbos left a comment

Choose a reason for hiding this comment

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

This is cool, great work. 2 comments, but will merge in for the sake of Hacktoberfest deadline.

<h2>{{name}}</h2>
<p>You have {{pr_count}} out of 4 PRs completed this October.</p>
<p><strong>{{statement}}</strong></p>
%if 'prs' in locals():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need locals(), seems you're passing all data into the template?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've probably misunderstood how the templates work… I must admit I faffed around with lots of different things before trying this (that I found on stackoverflow). I was trying to stop it attempting to print the table when there were no PRs to print… and now I'm looking at it again I should have just tested for pr_count > 0. Doh!

Thanks Bob.

pr_list.totalCount = 0

# We're going to pick out the details for each PR that we want to display to the user
pr_detail = {"url": [], "title": [], "date": []}
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about a list of named tuples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried with a namedtuple and got a bunch of errors in the template—the template seems to be very fussy on some things, I put it down to it being a beta version for the Py3 compatibility.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok gotcha, all good then, merged quickly as last day of Hacktoberfest :)

@bbelderbos bbelderbos merged commit 4289f64 into pybites:community Oct 31, 2022
@GeoffRiley GeoffRiley deleted the PCC38 branch October 31, 2022 15:06
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.

2 participants