Re: Proposal to make EccCheck non-destructive


Laszlo Ersek
 

On 10/05/20 01:25, Bret Barkelew via groups.io wrote:
In regards to this bug:
2986 – EccCheck should not revert staged and local changes (tianocore.org)<https://bugzilla.tianocore.org/show_bug.cgi?id=2986>

I have a proposal:

1. git diff --name-only <starting_commit>..<ending_commit/HEAD>
2. Copy list of changed files to a temp directory under Build/
* Perhaps something like Build/EccCheck or Build/EccCheckTemp
3. Remove files/directories that are in the exclusion list
4. Run EccCheck against that directory
5. Filter the report to remove the temp directory path so that the reported errors are root-based paths

The trade off of this approach is that EccCheck will run against multiple patches in aggregate, but – frankly – this isn’t any different than the other CI plugins.

The destructive nature of this plugin has caused headaches for everyone on our team that has worked with the new codebase. We all use local CI to validate changes as we make them, and having to make “test commits� just to ensure our work doesn’t go away isn’t a satisfactory process.

Thoughts? If this passes muster, I’m happy to code up some quick patches (if for no other reason than we can stage them in Mu while we’re waiting for full adoption).
The idea looks fine to me, but can you please clarify how it would be a
"trade-off"? That is -- does EccCheck (and if so, how) currently check
every patch in a series? I didn't know that.

(I don't think it's a property we "must" preserve in CI, just asking.)

Thanks
Laszlo

Join devel@edk2.groups.io to automatically receive all group messages.