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.

Sign up for GitHub

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

Jump to bottom

Convert CopyPasteManager to runtime class #766

Merged
merged 5 commits into from
Nov 13, 2019
Merged

Convert CopyPasteManager to runtime class #766

merged 5 commits into from
Nov 13, 2019

Conversation

rudyhuyn
Copy link
Contributor

@rudyhuyn rudyhuyn commented Nov 5, 2019

Fixes #765

Description of the changes:

  • convert CopyPasteManager to ref class
  • replace std::vector by Windows::Foundation::Collections::IVector
  • replace concurrency::task by Windows::Foundation::IAsyncOperation
  • replace static fields by static properties
  • refactor all methods having arguments passed by reference

Also:

  • code quality: use a new enum NumberBase to replace the int representing the number base
  • add CopyPasteManager::IsErrorMessage to check if a message is an error message (so we don't have to expose PasteErrorString

How changes were validated:

  • unit tests + manually

@mcooley mcooley added codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability) fixing approved issue labels Nov 6, 2019
@rudyhuyn
Copy link
Contributor Author

git rebase

EriWong
EriWong previously approved these changes Nov 13, 2019
View reviewed changes
Copy link
Contributor

@EriWong EriWong left a comment

Choose a reason for hiding this comment

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

Minor nit around filters to keep our folder structures in VS clean, but other than that, it looks good.

EriWong
EriWong approved these changes Nov 13, 2019
View reviewed changes
Copy link
Contributor

@EriWong EriWong left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@rudyhuyn rudyhuyn merged commit b9b0e06 into microsoft:master Nov 13, 2019
@rudyhuyn rudyhuyn deleted the Fix765 branch November 13, 2019 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EriWong EriWong EriWong approved these changes

Assignees

@EriWong EriWong

Labels
codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability) fixing approved issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Convert CopyPasteManager to runtime class
3 participants
@rudyhuyn @EriWong @mcooley