removed/replaced todo comments #2679
Conversation
| @@ -46,7 +46,9 @@ idPToMem = evState . evEff | |||
| evEff = reinterpret @_ @(State TypedState) $ \case | |||
| InsertConfig iw -> | |||
| modify' (insertConfig iw) | |||
| NewHandle _tid -> pure $ IdPHandle "IdP 1" --todo(leif): generate a new handle | |||
| NewHandle _tid -> | |||
| -- Same handle for all IdPs is good enough, for now | |||
There was a problem hiding this comment.
Can we elaborate on this? Why is a hardcoded handle fine?
There was a problem hiding this comment.
Because this is just a label shown to the team admin in the UI. IMO uniqueness is irrelevant for tests,
There was a problem hiding this comment.
👍 we'll see right away if this is not enough any more in the future, and then we can build something more sophisticated.
| @@ -46,7 +46,9 @@ idPToMem = evState . evEff | |||
| evEff = reinterpret @_ @(State TypedState) $ \case | |||
| InsertConfig iw -> | |||
| modify' (insertConfig iw) | |||
| NewHandle _tid -> pure $ IdPHandle "IdP 1" --todo(leif): generate a new handle | |||
| NewHandle _tid -> | |||
| -- Same handle for all IdPs is good enough, for now | |||
There was a problem hiding this comment.
👍 we'll see right away if this is not enough any more in the future, and then we can build something more sophisticated.
it would not have been more work to write the changelog entry instead of this line. and it wouldn't have prompted me to make things worse! :-) anyway, fine. |
IMO not a matter of which is more or less work, but rather what provides more value. Do we really need to document this in a changelog? What are the benefits? And how do they weigh against unnecessary noise in the changelog? |
Replaced some "todo" comments with something more meaningful.
No changelog necessary, IMO.
Checklist
changelog.d