Andromeda v4.6.7 (Clang Formatting) #86
Conversation
Jgerbrandt
changed the title
|
@shivamdesai04 and @EtSubas Let me know if you have any objections or suggestions for the styling options. |
| static void RunTask(void* pvParams) { | ||
| UARTTask::Inst().Run(pvParams); | ||
| } // Static Task Interface, passes control to the instance Run(); |
There was a problem hiding this comment.
If possible, try to make functions that are a single line inside the class definition stay a single line... though that's oddly specific so 🤷
There was a problem hiding this comment.
It will do this already if it does not exceed the column limit. The issue here is the comment exceeds the limit.
| bool CopyDataToCommand(uint8_t* dataSrc, | ||
| uint16_t size); // Copies the data into the command, into newly allocated memory |
There was a problem hiding this comment.
Change this to one line if possible.
Maybe note down what clang format option this is, if possible, because it's looks quite bad with how we've previously done comments.
There was a problem hiding this comment.
Again this is just due to the column limit. We might want to rethink our commenting structure or we disable the column limit.
EtSubas
left a comment
EtSubas
left a comment
There was a problem hiding this comment.
couple things none super important most are just small quality things
| while (!LL_USART_IsActiveFlag_TXE(kUart_)) {} | ||
| } | ||
| // Wait until the TX Register Empty Flag is set | ||
| while (!LL_USART_IsActiveFlag_TXE(kUart_)) {} |
There was a problem hiding this comment.
why is there a space here, looks weird
while (..) {
^ space
There was a problem hiding this comment.
what's your issue here. The only thing that changed is the tab spacing.
| private: | ||
| bool bShouldFreeData; // Should the Command handle freeing the data pointer (necessary to enable Command object to handle static memory ptrs) | ||
| private: | ||
| bool |
There was a problem hiding this comment.
weird that this is two lines I feel like the commit should be on another line instead of the var name
There was a problem hiding this comment.
Due to column limit. Unfortunately no way to change what gets moved to next line.
| /* Constants and Macros -------------------------------------------------------*/ | ||
| constexpr uint16_t DEFAULT_FLASH_SECTOR_SIZE = 4096; // 4KB - Default If Flash was not Initialized (to prevent % by 0 errors) | ||
| constexpr uint16_t DEFAULT_FLASH_SECTOR_SIZE = | ||
| 4096; // 4KB - Default If Flash was not Initialized (to prevent % by 0 errors) |
There was a problem hiding this comment.
nit: idk this seems ok but a little weird, I like to have the comment above instead... kinda just preference though so not a huge issue.
| (UBaseType_t)FLASH_TASK_RTOS_PRIORITY, | ||
| (TaskHandle_t*)&rtTaskHandle); | ||
|
|
||
| BaseType_t rtValue = xTaskCreate((TaskFunction_t)FlashTask::RunTask, (const char*)"FlashTask", |
There was a problem hiding this comment.
should this have function call on same line and have the args be pushed to a different line? seems like its not consistent
There was a problem hiding this comment.
If the line is short enough it will make it one line. Fine imo.
Clang Formatting
Adding Google style clang-formatting on pull request for the Components directory.
Adding .DS_Store to the gitignore for MacOS.