-
Notifications
You must be signed in to change notification settings - Fork 136
[Not for merge] News87: PR Review Club summaries #357
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
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.
High-level review looks pretty good. I get a word count of 144 on the bullet, which would put the full section (sans intro, which I think should be shorter for any subsequent editions) around 600-750 words per month. I think that's a bit long; I suggest trying to keep the average meeting bullet under 100 words to keep the whole section around 400 to 500 words. (I think 100 words is easily achievable here: truncating the quote and cutting the middle out of the following sentence gets you most of the way there.)
Also, I suggest that we add "PR" to the unintroduced abbreviations section of the style guide; that'll improve pacing and lower word count, and it looks like we've used that abbreviation over 150 times already on the site so it's nothing new.
Bitcoin Core reference implementation, and the process of making changes to | ||
Bitcoin. Notes, questions and meeting logs are posted on the website for each | ||
meeting for those who are unable to attend in real time, and as a permanent | ||
resource for those wishing to learn about the Bitcoin Core development process. |
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 think this is a fine introduction for the first time we do these summaries, but I wanted to note that, if we make this a regular monthly feature, I'll want a shorter one or two sentence version like the intros to our other monthly sections.
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.
Absolutely agree!
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.
Suggested wording if this becomes a monthly feature:
The Bitcoin Core PR Review Club is a weekly IRC meeting for newer Bitcoin Core contributors to learn about the project and the review process. In this monthly section, we summarize some recent Review Club meetings.
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.
@jnewbery love it. ACK! ❤️
68c990a
to
636ccf9
Compare
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.
Concept ACK!
Did some cutting and got it down to 115 words. Do you think that's too long?
Agree. Done in a separate commit. |
636ccf9
to
ceb33e2
Compare
That sound good. I think aiming for 100 words per bullet is a good guideline, but it shouldn't be a hard rule. |
ceb33e2
to
da21ea9
Compare
Fixed all review comments. I'm going to close this PR now and push a commit to Dave's PR when he opens it. |
Throwing this up for early review and feedback. This PR is not for merge, and this section will be incorporated into Dave's News87 PR when it gets opened.
@harding suggested including these summaries in IRC:
I think this also addresses @jonatack's suggestion of highlighting review in #301.
Looking for feedback on length/style of this first summary. If y'all like it I'll write similar summaries for the other PR Review Club meetings this month.