PX4-Autopilot icon indicating copy to clipboard operation
PX4-Autopilot copied to clipboard

Add check for missing or duplicate newlines at the end of files

Open MaEtUgR opened this issue 1 year ago • 3 comments

Solved Problem

When checking changes I realized people do commit missing or duplicate newlines every now and then. This seems like a non-issue and only leads to problems later on when some tooling doesn't work because the files are not POSIX compliant or whitespace checks fail in the middle of a merge. Manual passes like https://github.com/PX4/PX4-Autopilot/pull/22476 end up not enforcing the rules.

Solution

  • Add an automatic check for one single newline at the end of every file that is checked into the repository.

Changelog Entry

Style: Automate enforcing one single newline at the end of each file

Alternatives

Ideally, this would also run through with make format but I don't add it because I feel it adds too much delay and hence prolongs the pre-commit hook. Better to have it ran in CI than not at all but maybe someone knows how to make the check more efficient such that the runtime is insignificant during commits.

Test coverage

I tested locally that it triggers with:

  • All the existing missing newlines and duplicate newlines
  • Just one of the two checks
  • Just one duplicate newline in a file

Context

Inspiration:

  1. https://stackoverflow.com/a/54348931
  2. https://stackoverflow.com/a/31829703/6326048

MaEtUgR avatar May 22 '24 12:05 MaEtUgR

The check fails when not compliant: https://github.com/PX4/PX4-Autopilot/actions/runs/9191515223/job/25278084826?pr=23166

MaEtUgR avatar May 22 '24 12:05 MaEtUgR

The check passes after the fixes image

MaEtUgR avatar May 22 '24 13:05 MaEtUgR

Hmm, e.g. SITL offboard, px4_fmu-v6xrt, MAVROS rover mission CI is also failing on main. There are quite some to look through 👀

MaEtUgR avatar May 22 '24 14:05 MaEtUgR

I rebased the pr, this would have caught more newline issues since I opened the pr: image

e.g. https://github.com/PX4/PX4-Autopilot/pull/22904/files#r1681278804

MaEtUgR avatar Jul 17 '24 15:07 MaEtUgR

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/px4-sync-q-a-july-17-2024/39749/1

DronecodeBot avatar Jul 17 '24 16:07 DronecodeBot