-
Notifications
You must be signed in to change notification settings - Fork 438
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
views: moderation: Add unit tests, proper response instead of "Yo", apiDoc clarifications #805
Conversation
9be94eb
to
4cc00b7
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.
Nice effort! Some minor nits here and there.
Please also keep commit message titles under 50 chars (or at least under 70, if otherwise not possible), else GitHub will hard-break the title with ellipses.
4cc00b7
to
24fb28c
Compare
Thanks for the review 🎉 i addressed all your feedback, take an other look if you spot any issues Except the thing with the git commit message length, im not to sure what i should do there I can drop the prefix to make them shorter, but i guess the idea is to use prefixes? I can create super short unspecific commit messages which i feel would be worse. |
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.
Except the thing with the git commit message length, im not to sure what i should do there I can drop the prefix to make them shorter, but i guess the idea is to use prefixes? I can create super short unspecific commit messages which i feel would be worse. So i am a bit lost how i should solve that any ideas?
Don't worry too much about that. Those commit message conventions are just that, conventions. I personally use abbreviations or more terse wording and kind of "force" myself to keep below 50 chars, as I've set my editor to "warn" me when I go over.
E.g.
views: comments: moderation: Replace Yo response with a more descriptive text
I'd replace with
views: comments: Proper response instead of 'Yo'
But again, that's just stylistic pet peeves. You'll notice the usefulness of short commit messages when browsing git logs and being annoyed at the cut-off messages ending in ellipses.
Thanks for those tests. One inline comment, everything else looks marvelous!
The "key" does _contain_ a base64-encoded list of '["unsubscribe","<email>"]' but also a signature. As such, remove the comment to avoid confusion (above, it is mentioned where the key is obtained from). Also add the "unsubscribe" part to the request URL that was missing.
Verified using Thank you for your contribution! |
Things this PR addresses:
Contributes to #755