-
Notifications
You must be signed in to change notification settings - Fork 31
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
Add maximum expiry #62
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.
In general, looks good, just some questions.
thanks @HeapUnderfl0w ! 🎉 This certainly looks more sophisticated than what I did. |
76db6b3
to
a5e9363
Compare
Don't worry, I wasn't outright rejecting your proposal. A few more iterations and it would've landed. But yes, let's go with this one here. |
d00b84b
to
bea0dc2
Compare
This should be all of the concerns addressed. |
@matze currently the json endpoints silently clamp the expiration value to the maximum. Would you want to error out of here too? (some kind of http error) |
bea0dc2
to
ecd9822
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.
Just the tiny end user clarification, rest looks good.
Yes, keep as is for now. |
expiry -> expiration hard fail on invalid expiration values
ecd9822
to
72e1d83
Compare
This aims to implement #54, and also be an improvement over #55.
Changes Made:
WASTEBIN_MAX_PASTE_EXPIRY
WASTEBIN_MAX_PASTE_EXPIRATION
Currently it simply clamps the expiry timenever expire
if any limit is setburn
, even if the limit is 0