-
Notifications
You must be signed in to change notification settings - Fork 337
feat(publish): better error msg on needs email verif #795
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.
tiny nit on messaging but lgtm
2f6cc93
to
191a984
Compare
@EverlastingBugstopper is it better now? feels awk to me but i dunno what to update it to |
3940cb5
to
dce73eb
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.
lgtm
dce73eb
to
75d4a84
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.
Small maintainability questions/suggestions. If neither looks compelling to you, we can go ahead with merge!
@@ -122,6 +121,33 @@ fn build_and_publish_script( | |||
Ok(()) | |||
} | |||
|
|||
fn error_msg(status: reqwest::StatusCode, text: String) -> String { | |||
if text.contains("\"code\": 10034,") { |
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.
We could parse the text into a json blob if we wanted and allow for pretty formatting of error codes (e.g. access error codes directly instead of doing text.contains()
). With that, we could match on error code value and elegantly print out helpful messages specific to error codes (for code 10034 and beyond)!
That being said, there's no immediate case for json parsing and handling here--the text approach is appropriate for our current error handling needs.
}"# | ||
.to_string(); | ||
let result = error_msg(status, text); | ||
assert!(result.contains("https://dash.cloudflare.com")); |
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.
Could we even make the error message above a string const and compare the actual result in this test to this string const?
@gabbifish i think the idea of actually parsing the error to make it prettier would be a great next step- i'll file an issue! |
fixes #320