Incorrect include guard in BookChanges.h (Version: 1.9.1++)

[scottschurr]

Issue Description

The include guard for src/ripple/rpc/BookChanges.h looks like this:

#ifndef RIPPLE_RPC_BOOKCHANGES_H_INCLUDED
#define RIPPLE_RPC_BOOKCAHNGES_H_INCLUDED

The #define needs to be spelled the same way as the #ifndef in order to be effective.

Thanks to clang warnings for pointing this out. But I also can no longer build with warnings as errors with this mistake in the code base.

I'll also note that It's unfortunate that I cannot associate this change with a particular version of the code. There seem to be new commits (this being one of them) that have entered Develop without being associated with a version change. Is this what we should expect for the future?

[RichardAH]

We’re discussing in MM the best way forward with develop branch. We also need better CI to prevent simple typos making it past review into code

[RichardAH]

@scottschurr Can you please PR an update to reinstate builds as part of CI. Two reviewers missed this typo, where CI would have caught it. I don't understand why CI builds were disabled, we used to have CI builds right?

[scottschurr]

@RichardAH, yes, I suspect CI builds would have caught this. Yes, we had CI builds for years. However our CI builds have been dead for months due to problems with Travis.

I hear there have been people working on a different way to do CI builds, but apparently they are not done yet. I hope they get something running very soon.

Additionally, historically there has been a ritual around version releases to develop that has made the lack of CI builds less of a problem. Typically when a new (tagged) version of develop is proposed we get developers on Linux, Windows, and Mac to all build and unit test the proposed version. They report if they see problems. We don't commit the proposed update to develop until all platforms have weighed in. If that ritual had been followed the bad include guard probably would have been caught before it was committed to develop.

[MarkusTeufelberger]

It makes me wonder if people who develop this code compile it without setting -Werror or similar?

[scottschurr]

@MarkusTeufelberger, that's a fair thing to suspect. It's possible that only clang generates this warning? So maybe the code was committed to develop before being compiled with clang and -Werror. There are certainly compilers other than clang that don't warn about this issue.

[MarkusTeufelberger]

Maybe In should update and port the stuff from https://github.com/MarkusTeufelberger/rippled-distrotest over to github actions or similar... 🤔

[RichardAH]

It's compiled with the available build engine under default settings to test. I agree it should be changed to catch this