LRS-48 LRS-49 Prep for OSS Release

yetanalytics / lrs

Protocols, specifications, and logic for building an xAPI Learning Record Store (LRS) in Clojure(Script).

https://www.yetanalytics.com/lrs

Apache License 2.0

4 stars 1 forks source link

LRS-48 LRS-49 Prep for OSS Release #50

Closed milt closed 3 years ago

milt commented 3 years ago

LRS-48 Let's open lrs up!

Clears out a bunch of old code, updates to the now-public LRS test runner, and sets up GH actions!

NOTE: We should merge this and get other open PRs synced up before OSS release

We can put status badges anywhere now lol

kelvinqian00 commented 3 years ago

I notice that you still have some commented out code here: https://github.com/yetanalytics/lrs/blob/LRS-49/src/main/com/yetanalytics/lrs/pedestal/interceptor/xapi/statements/attachment.cljc#L246

Also here (though looking at your changes it appears you left this alone on purpose): https://github.com/yetanalytics/lrs/blob/LRS-49/src/dev/mem_lrs/server.cljc#L126

milt commented 3 years ago

I notice that you still have some commented out code here: https://github.com/yetanalytics/lrs/blob/LRS-49/src/main/com/yetanalytics/lrs/pedestal/interceptor/xapi/statements/attachment.cljc#L246

Indeed. I left some commented stuff in a few cases:

"Rich Comment" blocks like this one that might be useful to explain usage
Boilerplate comments like the servlet stuff that could be used for further dev
A few instances in dev paths where it was illustrative to keep the commented-out code

milt commented 3 years ago

But I made it a priority to remove commented out forms in hot code paths, which was my big concern from a maintainability standpoint

kelvinqian00 commented 3 years ago

Ok, in that case if I were you I'd put a bit of effort in making the commented-out code look a bit nicer (because right now some of it still looks like chicken scratch).