Hello all,

Recently we have been getting a number of large patch sets. Please note that a large patch set often makes it harder to review. It also overwhelms the CI which submits multiple jobs for each patch submitted (and updated). I’m sorry if I’m not getting to review all the patches, there are a lot. And my GerritHub dashboard is getting unwieldy…

If I could make some suggestions on consolidating patches:

If you are introducing a bunch of new functions or config parameters and have patches that provide those, but utilize them in a separate patch, consider if you can consolidate all the new stuff into a single patch.

In general, try and consolidate changes, especially small ones. A major change, even if other changes are related, may well make sense to be in a separate patch.

It is nice when a patch doesn’t make a second change to lines changed in a previous patch.

If you are making a FSAL API change that is small, consider one patch to update all the FSALs. On the other hand, for a larger change, one patch per FSAL makes it easier to assure that each FSAL’s stake holders have reviewed the change. In this case, a larger patch set is understandable.

If you have patches that fix bugs or other stuff that isn’t exactly pertinent to your changes, it can be helpful to have those patches at the front of your patch set (farthest from HEAD). I have sometimes merged these while still discussing the rest of the patch set.

On another note, I am seeing a lot of checkpatch failures, please make sure you have the checkpatch commit hook installed as per CONTRIBUTING_HOWTO.txt. One of the patch sets submitted is a suggestion to change to clang, which I am in favor of, and we will make that change when the dust settles some. Once we make that change, the commit hook will change, but until then, please conform to checkpatch.

Thanks

Frank