fix: don't check storage for new uaid's #1020
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1020 +/- ##
=======================================
Coverage 99.78% 99.78%
=======================================
Files 57 57
Lines 9598 9598
=======================================
Hits 9577 9577
Misses 21 21Continue to review full report at Codecov.
|
| @@ -359,8 +361,9 @@ def lookup_user(self, hello): | |||
| # type: (Hello) -> (Optional[JSONDict], JSONDict) | |||
| flags = dict( | |||
| message_month=None, | |||
| rotate_message_table=False, | |||
| check_storage=False, | |||
There was a problem hiding this comment.
nit: can we not change the order of these? It's not like they're in alphabetical order anyway, and it's generally better to put new items at the end of lists to avoid accidental flag set.
There was a problem hiding this comment.
When creating a dict? I understand it in keyword args to API methods to add it to the end, but this just makes a dict.
There was a problem hiding this comment.
fair, but since we do specify a set of args that match these, I thought it might be nice for consistency.
autopush_rs/src/client.rs
Outdated
| => self.data.process_hello(uaid, message_month, reset_uaid, rotate_message_table), | ||
| _ => return Err("Already connected elsewhere".into()), | ||
| call::HelloResponse { uaid: Some(uaid), message_month, check_storage, reset_uaid, rotate_message_table } | ||
| => self.data.process_hello(uaid, message_month, check_storage, reset_uaid, rotate_message_table), |
There was a problem hiding this comment.
nit: this seems hard to read quickly. Should there be an indent here to designate that call::HelloResponse is the match?
There was a problem hiding this comment.
I have an upcoming PR that will run rustfmt over all of it, which will reformat this. I didn't want to fold that into this as there's lots of little tweaks in it.
autopush_rs/src/client.rs
Outdated
| @@ -435,9 +435,10 @@ where | |||
| Ok(Async::Ready(item)) | |||
| } | |||
|
|
|||
| fn process_hello(&mut self, uaid: Uuid, message_month: String, reset_uaid: bool, rotate_message_table: bool) -> ClientState { | |||
| fn process_hello(&mut self, uaid: Uuid, message_month: String, check_storage: bool, reset_uaid: bool, rotate_message_table: bool) -> ClientState { | |||
There was a problem hiding this comment.
nit: Again, I'd be happier if the new arg was at the end of this list.
There was a problem hiding this comment.
Ah yes, here it matters! Changing.
bbangert
force-pushed
the
feat/issue-1017
branch
from
b30efc6 to
922e342
Compare
Closes #1017