-
Notifications
You must be signed in to change notification settings - Fork 17
Fix typos in comments #28
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.
Thanks a lot, great catches!
@pascalkuthe We'd need your help here - there is a clippy fix without which no PR will be able to merge, and I can't self-approve. The alternative should be that @Hawk777 closes #29 and puts that commit here, to mine isn't the topmost one anymore, maybe? |
If the fix to the Clippy lint can go into master, I can rebase both my branches. |
Your fix looks reasonable to me, but as I’m not a maintainer of this repo, I probably shouldn’t bypass the rules by cherry-picking it into my branch or doing other similar hacks. |
This setting is just a maintenance hassle - if I wanted to mess with the repo I could just use another account and merge whatever I want, right? @Hawk777 It seems like if you would push last, for instance by fixing clippy, I can merge this. |
@Byron alright, I think it’s trivial enough that I’m fine with cherry-picking your commit into a PR of my own, which is GH-30 (I’ll rebase my other two after it’s merged). With respect to “This setting is just a maintenance hassle - if I wanted to mess with the repo I could just use another account and merge whatever I want, right?” it makes sense if someone has commit access but doesn’t have the ability to grant other people access; then they couldn’t add a second of their own accounts, so they’d be forced to follow the rule. |
You can supersede this PR with a new one that doesn't contain my commit. |
This PR already doesn’t contain your commit. I’ll just rebase it. |
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.
LGTM
Recreated as GH-31. |
No description provided.