Export so that it can be consumed elsewhere
Depends on D4297
Details
Details
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Resigning since I'm usually not a good person to set as a first-pass reviewer. Exceptions here
| native/cpp/lib/sqlite_orm/CMakeLists.txt | ||
|---|---|---|
| 18 ↗ | (On Diff #14299) | We should capitalize the first letter to be consistent with the other comments |
Comment Actions
@jonringer-comm, do we have a task for adding a CMake linter? I think this is the 6th time I've left rote comments like this about things like line length, newlines, and indentation. Seeing as it's not something you're reliably able to catch ahead of submitting diffs, it's critical that you prioritize integrating a CMake linter into the project. Until you do that, pretty much all of your diffs will require repeated back and forth, which wastes your time and your reviewers' time. Please take this comment seriously this time!
| native/cpp/lib/sqlite_orm/CMakeLists.txt | ||
|---|---|---|
| 36 ↗ | (On Diff #14299) | Line is 86 chars long |
| native/cpp/lib/sqlite_orm/CMakeLists.txt | ||
|---|---|---|
| 18 ↗ | (On Diff #14299) | Do you mean like Comm-sqlite-orm ? because if we go with PascalCase, then we should probably do CommSqliteOrm. If we do, then I would ask that we do that refactor after all of the in-flight work is done. Having to juggle ~12 diffs is really difficult, and that's something we can do relatively easily in a later diff. |
Comment Actions
@atul just making sure that you're still okay with doing project name refactoring at a later time
| native/cpp/lib/sqlite_orm/CMakeLists.txt | ||
|---|---|---|
| 18 ↗ | (On Diff #14299) | Oh, I was just referring to capitalizing the "r" in "reference" |
Comment Actions
@atul just making sure that you're still okay with doing project name refactoring at a later time
Was suggesting that we capitalize the first letter of the comment, not that we needed to capitalize the project name.
(I do slightly prefer PascalCase, but that wasn't what I was suggesting here)
| native/cpp/lib/sqlite_orm/CMakeLists.txt | ||
|---|---|---|
| 18 ↗ | (On Diff #14299) | oh, that's what I at first, but noticed it was for the line below... was just making sure :) |