Remove deprecated pict from user profile - #5538
VeryMilkyJoe wants to merge 6 commits into
Conversation
2599f55 to
c61509b
Compare
zebot
added
the
ok-to-test
c61509b to
81d2a73
Compare
Starting with V18, UserProfile does not contain profilePict anymore, while for older versions the old schema will still be produced.
81d2a73 to
c75fe89
Compare
| :> "users" | ||
| :> QualifiedCaptureUserId "uid" | ||
| :> GetUserVerb | ||
| :> GetUserVerb 'V18 |
There was a problem hiding this comment.
I was wrong when we talked earlier: v18 has already been finalized, so you can't change it any more. Please 's/v17/v18/g && s/v18/v19/g' everywhere.
There was a problem hiding this comment.
Also, shouldn't you not have a version here, given that the instance you want to match isn't instance ToSchema (Versioned 'V18 UserProfile), but instance ToSchema UserProfile?
There was a problem hiding this comment.
Isn't it the former I want to match?
I defined the toSchema instance as e.g. instance ToSchema (Versioned 'V18 UserProfile) or am I misunderstanding you?
There was a problem hiding this comment.
Sorry again, I saw the instance ToSchema UserProfile where that you didn't delete, and failed to see the V18 one.
So now that I squinted a little more at it everything makes sense, except that maybe the ToSchema UserProfile instance is redundant? And I wonder if I would prefer to keep UserProfile and remove V18-UserProfile, making the route look like before? after all it's not only v18, it's just everything from now on.
There was a problem hiding this comment.
Yes, that sounds very reasonable, thank you for the suggestion!
fisx
left a comment
fisx
left a comment
There was a problem hiding this comment.
One more nit that you can consider, but I think it's ready to merge as it is! 馃憤
| case mVersion of | ||
| Just v | ||
| | v > V18 -> pure noPict | ||
| _ -> const noPict .= (field "picture" schema <|> pure noPict) |
There was a problem hiding this comment.
I just realized that const noPict generates the wrong FromJSON instance: it reads the field, but ignores its contents and puts in noPict.
We're only interested in ToJSON really, and the pict field hasn't been used in forever. But I think it's a nice puzzle to fix this.
3 participants
Starting with V18, UserProfile does not contain profilePict anymore, while for older versions the old schema will still be produced.
Todo:
Checklist
changelog.d