Ecosyste.ms: Timeline
Browse the timeline of events for every public repo on GitHub. Data updated hourly from GH Archive.
jlkravitz created a review comment on a pull request on csg-org/CompactConnect
Got it -all good. Thanks for the explanation!
landonshumway-ia created a review comment on a pull request on csg-org/CompactConnect
Good call out, initially I had it as an optional field, but there's no reason it shouldn't be required. All of our current configuration files have it included, so I will remove this check and we'l...
ChiefStief created a review comment on a pull request on csg-org/CompactConnect
@jlkravitz Sorry I was just confirming why we do this. This is something we do across the codebase. This is just a sanity thing so that we can cleanly define what the properties are going to be ...
jlkravitz created a review comment on a pull request on csg-org/CompactConnect
If an option, why not store the version as an integer rather than as a string?
jlkravitz created a review comment on a pull request on csg-org/CompactConnect
Could this ever happen? Is `attestations` not a required field?
jlkravitz created a review comment on a pull request on csg-org/CompactConnect
What does this comment mean? (What is 'this'?)
jusdino created a review comment on a pull request on csg-org/CompactConnect
Yes, your understanding is correct. I don't see any cases where that would not be true for our data model but, now that you point it out, I don't think I want this limitation hiding in the code her...
jusdino created a review comment on a pull request on csg-org/CompactConnect
Yeah, we can talk through separator and b64 alphabet character choice when we do a design view. Ultimately, it doesn't matter a lot what we choose for separators vs 62nd, 63rd b64 characters, as lo...
jusdino created a review comment on a pull request on csg-org/CompactConnect
Yeah, good call out. We can do a team design review to mull this over before committing to it.
jusdino created a review on a pull request on csg-org/CompactConnect
Great! Ready for you, @jlkravitz .
landonshumway-ia created a review comment on a pull request on csg-org/CompactConnect
same here: `_populate_update_record`
landonshumway-ia created a review comment on a pull request on csg-org/CompactConnect
nit: should start with underscore if we don't intend to call it outside of this file: `_process_license_update`
landonshumway-ia created a review comment on a pull request on csg-org/CompactConnect
Just need to check my understanding here: with the way this is structured, it suggests that the base license and privilege records will ALWAYS come first in the array before any update records for ...
landonshumway-ia created a review comment on a pull request on csg-org/CompactConnect
This right here makes me wonder if we should consider choosing other separators if we need to have logic like this... Are there not other options we could use that wouldn't require this?
landonshumway-ia created a review comment on a pull request on csg-org/CompactConnect
This is a pretty critical piece, as whatever pattern we choose we will essentially be locked into forever to determine the update records by hash. I am trying to walk through any drawbacks with thi...
landonshumway-ia created a review comment on a pull request on csg-org/CompactConnect
We should also have a log statement showing that we are restoring the record and what the change would have been. Given that we don't expect this code to get exercised very often, we should be fine...
landonshumway-ia created a review comment on a pull request on csg-org/CompactConnect
missing a error log message here that would be useful for context
landonshumway-ia created a review comment on a pull request on csg-org/CompactConnect
it may be helpful to have a log message here noting that we are deleting something, so we at least have some paper trail of what record would have shown up in the system
landonshumway-ia created a review comment on a pull request on csg-org/CompactConnect
We should leave a comment here explicitly calling out why we append the provider privilegeJurisdictions update last, so no one ever moves it to an earlier step in the algorithm.
jlkravitz created a review comment on a pull request on csg-org/CompactConnect
@ChiefStief Flagging this before we merge. If this is something we've been doing across our codebase, we can ignore!