Skip to content

Conversation

@sfc-gh-tclinkenbeard
Copy link
Collaborator

@sfc-gh-tclinkenbeard sfc-gh-tclinkenbeard commented Jan 30, 2021

This fixes the Windows CI by undoing a change to BackupType enum names.

General guideline:

  • If this PR is ready to be merged (and all checkboxes below are either ticked or not applicable), make this a regular PR
  • If this PR still needs work, please make this a draft PR
    • If you wish to get feedback/code-review, please add the label RFC to this PR

Please verify that all things listed below were considered and check them. If an item doesn't apply to this type of PR (for example a documentation change doesn't need to be performance tested), you should make a strikethrough (markdown syntax: ~~strikethrough~~). More infos on the guidlines can be found here.

Style

  • All variable and function names make sense.
  • The code is properly formatted (consider running git clang-format).

Performance

  • All CPU-hot paths are well optimized.
  • The proper containers are used (for example std::vector vs VectorRef).
  • There are no new known SlowTask traces.

Testing

  • The code was sufficiently tested in simulation.
  • If there are new parameters or knobs, different values are tested in simulation.
  • ASSERT, ASSERT_WE_THINK, and TEST macros are added in appropriate places.
  • Unit tests were added for new algorithms and data structure that make sense to unit-test
  • If this is a bugfix: there is a test that can easily reproduce the bug.

@sfc-gh-tclinkenbeard sfc-gh-tclinkenbeard marked this pull request as ready for review January 30, 2021 06:51
Copy link
Collaborator

@sfc-gh-mpilman sfc-gh-mpilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Trevor!

@sfc-gh-mpilman sfc-gh-mpilman merged commit 8211cf7 into apple:master Jan 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants