Skip to content
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.

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

UefiDbgExt: apply cpp uncrustify formatting #806

Closed
wants to merge 1 commit into from

Conversation

VivianNK
Copy link

Preface

Please ensure you have read the contribution docs prior
to submitting the pull request. In particular,
pull request guidelines.

Description

<Please include a description of the change and why this change was made.>

For each item, place an "x" in between [ and ] if true. Example: [x].
(you can also check items in the GitHub UI)

  • Impacts functionality?
    • Functionality - Does the change ultimately impact how firmware functions?
    • Examples: Add a new library, publish a new PPI, update an algorithm, ...
  • Impacts security?
    • Security - Does the change have a direct security impact on an application,
      flow, or firmware?
    • Examples: Crypto algorithm change, buffer overflow fix, parameter
      validation improvement, ...
  • [ x] Breaking change?
  • Includes tests?
    • Tests - Does the change include any explicit test code?
    • Examples: Unit tests, integration tests, robot tests, ...
  • Includes documentation?
    • Documentation - Does the change contain explicit documentation additions
      outside direct code modifications (and comments)?
    • Examples: Update readme file, add feature readme file, link to documentation
      on an a separate Web page, ...

How This Was Tested

<Please describe the test(s) that were run to verify the changes.>

Integration Instructions

<Describe how these changes should be integrated. Use N/A if nothing is required.>

@github-actions github-actions bot added impact:breaking-change Requires integration attention impact:non-functional Does not have a functional impact labels Dec 19, 2023
@makubacki
Copy link
Member

makubacki commented Dec 20, 2023

My understanding is that this code is largely derived and/or relatable to another source. Given that, it may be detrimental to maintenance to introduce unnecessary deviation. In addition, the Uncrustify rules are intended for firmware code so it's a bit ambiguous if they should apply here (to a WinDbg extension). I'm also not aware of an effort to actively run Uncrustify against this code in CI like the firmware code meaning the formatting will not be upheld over time losing its original purpose.

Without any additional context about the change, for those reasons, I lean toward not applying these Uncrustify rules to this code at this time.

@VivianNK VivianNK changed the title UefiDbgExt: applu cpp uncrustify formatting UefiDbgExt: apply cpp uncrustify formatting Dec 21, 2023
@VivianNK VivianNK closed this Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact:breaking-change Requires integration attention impact:non-functional Does not have a functional impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants