Skip to content

Add code formatting check to CI #3778

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 21 commits into from
Jun 24, 2025
Merged

Add code formatting check to CI #3778

merged 21 commits into from
Jun 24, 2025

Conversation

@thomas-bc
Copy link
Collaborator

@thomas-bc thomas-bc commented Jun 23, 2025

Related Issue(s)
Has Unit Tests (y/n)
Documentation Included (y/n)

Change Description

Adds code formatting check to CI
This only checks a few directories for now. The plan will be to walk through the repo a few directories at a time in multiple PRs, to make reviewing easier.

Fix #3474

Step towards #1984

github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 23, 2025
View reviewed changes
Fw/Cmd/CmdArgBuffer.cpp
FW_ASSERT(FW_SERIALIZE_OK == stat,static_cast<FwAssertArgType>(stat));
}
CmdArgBuffer::CmdArgBuffer(const U8* args, FwSizeType size) {
SerializeStatus stat = this->setBuff(args, size);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter args has not been checked.
Fw/Cmd/CmdArgBuffer.cpp
FW_ASSERT(FW_SERIALIZE_OK == stat,static_cast<FwAssertArgType>(stat));
}
CmdArgBuffer::CmdArgBuffer(const U8* args, FwSizeType size) {
SerializeStatus stat = this->setBuff(args, size);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter size has not been checked.
Fw/Cmd/CmdArgBuffer.cpp
return *this;
}
CmdArgBuffer::CmdArgBuffer(const CmdArgBuffer& other) : Fw::SerializeBufferBase() {
SerializeStatus stat = this->setBuff(other.m_bufferData, other.getBuffLength());

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter other has not been checked.
Fw/Cmd/CmdPacket.cpp
}

SerializeStatus CmdPacket::deserialize(SerializeBufferBase& buffer) {
SerializeStatus stat = ComPacket::deserializeBase(buffer);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter buffer has not been checked.
Fw/Com/ComBuffer.cpp
FW_ASSERT(FW_SERIALIZE_OK == stat,static_cast<FwAssertArgType>(stat));
}
ComBuffer::ComBuffer(const U8* args, FwSizeType size) {
SerializeStatus stat = SerializeBufferBase::setBuff(args, size);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter args has not been checked.
Fw/Com/ComBuffer.cpp
FW_ASSERT(FW_SERIALIZE_OK == stat,static_cast<FwAssertArgType>(stat));
}
ComBuffer::ComBuffer(const U8* args, FwSizeType size) {
SerializeStatus stat = SerializeBufferBase::setBuff(args, size);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter size has not been checked.
Fw/Com/ComBuffer.cpp
return *this;
}
ComBuffer::ComBuffer(const ComBuffer& other) : Fw::SerializeBufferBase() {
SerializeStatus stat = SerializeBufferBase::setBuff(other.m_bufferData, other.getBuffLength());

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter other has not been checked.
Fw/Com/ComPacket.cpp
return buffer.serialize(static_cast<FwPacketDescriptorType>(this->m_type));
}
SerializeStatus ComPacket::serializeBase(SerializeBufferBase& buffer) const {
return buffer.serialize(static_cast<FwPacketDescriptorType>(this->m_type));

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter buffer has not been checked.
Fw/Com/ComPacket.cpp
return stat;
SerializeStatus ComPacket::deserializeBase(SerializeBufferBase& buffer) {
FwPacketDescriptorType serVal;
SerializeStatus stat = buffer.deserialize(serVal);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter buffer has not been checked.
github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 23, 2025
View reviewed changes
Fw/Buffer/Buffer.cpp
m_context(src.m_context)
{
if(src.m_bufferData != nullptr){
Buffer::Buffer() : Serializable(), m_serialize_repr(), m_bufferData(nullptr), m_size(0), m_context(0xFFFFFFFF) {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Fw/Cmd/CmdArgBuffer.cpp

CmdArgBuffer::CmdArgBuffer() {
}
CmdArgBuffer::CmdArgBuffer() {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Fw/Cmd/CmdArgBuffer.cpp

CmdArgBuffer::~CmdArgBuffer() {
}
CmdArgBuffer::~CmdArgBuffer() {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Fw/Cmd/CmdPacket.cpp
}

SerializeStatus CmdPacket::serialize(SerializeBufferBase& buffer) const {
CmdPacket::~CmdPacket() {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Fw/Cmd/CmdPacket.cpp
return FW_SERIALIZE_OK; // for compiler
}

SerializeStatus CmdPacket::deserialize(SerializeBufferBase& buffer) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Fw/Com/ComBuffer.cpp

ComBuffer::ComBuffer() {
}
ComBuffer::ComBuffer() {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Fw/Com/ComBuffer.cpp

ComBuffer::~ComBuffer() {
}
ComBuffer::~ComBuffer() {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Fw/Com/ComPacket.cpp

ComPacket::ComPacket() : m_type(ComPacketType::FW_PACKET_UNKNOWN) {
}
ComPacket::ComPacket() : m_type(ComPacketType::FW_PACKET_UNKNOWN) {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Fw/Com/ComPacket.cpp

ComPacket::~ComPacket() {
}
ComPacket::~ComPacket() {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 23, 2025
View reviewed changes
Fw/Buffer/Buffer.cpp
#if FW_SERIALIZABLE_TO_STRING
void Buffer::toString(Fw::StringBase& text) const {
static const char * formatString = "(data = %p, size = %u, context = %u)";
static const char* formatString = "(data = %p, size = %u, context = %u)";

Check notice

Code scanning / CodeQL

Use of basic integral type Note

formatString uses the basic integral type char rather than a typedef with size and signedness.
@thomas-bc thomas-bc mentioned this pull request Jun 23, 2025
Closed
github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 23, 2025
View reviewed changes
Fw/Cmd/CmdArgBuffer.cpp
}
CmdArgBuffer::CmdArgBuffer(const CmdArgBuffer& other) : Fw::SerializeBufferBase() {
SerializeStatus stat = this->setBuff(other.m_bufferData, other.getBuffLength());
FW_ASSERT(FW_SERIALIZE_OK == stat, static_cast<FwAssertArgType>(stat));

Check warning

Code scanning / CppCheck

Member variable 'CmdArgBuffer::m_bufferData' is not assigned in the copy constructor. Should it be copied? Warning

Member variable 'CmdArgBuffer::m_bufferData' is not assigned in the copy constructor. Should it be copied?
Fw/Com/ComBuffer.cpp
}
ComBuffer::ComBuffer(const ComBuffer& other) : Fw::SerializeBufferBase() {
SerializeStatus stat = SerializeBufferBase::setBuff(other.m_bufferData, other.getBuffLength());
FW_ASSERT(FW_SERIALIZE_OK == stat, static_cast<FwAssertArgType>(stat));

Check warning

Code scanning / CppCheck

Member variable 'ComBuffer::m_bufferData' is not assigned in the copy constructor. Should it be copied? Warning

Member variable 'ComBuffer::m_bufferData' is not assigned in the copy constructor. Should it be copied?
@thomas-bc thomas-bc mentioned this pull request Jun 23, 2025
Closed
3 tasks
@thomas-bc
Copy link
Collaborator Author

thomas-bc commented Jun 23, 2025 •
edited
Loading

Ouch - looks like there's no pre-built version of https://pypi.org/project/clang-format/ for ARM (32bit). So our checks running RPI 32bit are slowed down because pip will try to build from source...

EDIT: fixed by requiring clang-format only when not on ARM32. ARM32 systems can install through other means, and most likely development (code formatting) is not happening on ARM32 anyways.

@thomas-bc thomas-bc requested a review from LeStarch June 23, 2025 21:39
@thomas-bc thomas-bc merged commit 1a07c84 into nasa:devel Jun 24, 2025
75 of 77 checks passed
@thomas-bc thomas-bc deleted the ci/formatting branch June 24, 2025 19:19
@thomas-bc thomas-bc added the Breaking Changes / Needs Release Notes Need to add instructions in the release notes for updates. label Aug 6, 2025
@thomas-bc
Copy link
Collaborator Author

thomas-bc commented Aug 6, 2025

Other Changes

  • Framework code has been entirely formatted for better readability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LeStarch LeStarch LeStarch approved these changes

Assignees

No one assigned

Labels

Breaking Changes / Needs Release Notes Need to add instructions in the release notes for updates.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Standard way to automatically verify that F Prime code formatting is correct in CI

2 participants

@thomas-bc @LeStarch