-
-
Notifications
You must be signed in to change notification settings - Fork 191
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
Ensure readmes are updated #778
Conversation
I admit being a little perplexed by this statement, given the existence of #777, but I'll roll with it. So if you would like these commits merged, I would like to ask that each commit contains references to the problem-specifications PRs that caused the changes. If that sounds like too much work, well for this particular case, lucky for you, since I already did it, but if it sounds like too much work for the future, we should talk about that! |
Thanks for your patience. |
Well, no worries, you statement of
is undoubtedly true, so that could help prepare you for later. And the knowledge should be applicable to your other tracks as well. |
That first commit in this PR could be useful to have though. So if you want that, you could consider reopening this PR and putting only this commit on there! |
Will do. |
- Make file executable - Check for `configlet` in PATH first.
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.
Hmm, there may be an argument that this should be two commits, but I guess they can be lumped under one commit with the message "make bin/ensure-readmes-are-updated more conveniently runnable locally" or something like that
I'm sorry, I pushed the button too fast. |
Was there a problem? Did you want to do something before pushing the button? |
Oh, I guess if it's about the commit messages and/or PR title - that is not really so terrible. But it is true that sometimes I will take an action along the lines of "Approve, assuming you make these changes before merging" versus "Approve, and no changes are needed", and I can do a better job of distinguishing between these cases? In this case, I didn't really care too much. |
I'll read the message on PC, that should be sufficient. :-) |
I didn't want to beat you to it, @petertseng, since you offered to do this.
But I figured this would be a good exercise for me to get to know the maintainer pipeline.
Fixes build problem in #773.