Skip to content

(Updated) Wires: New icon resources added.#12006

Merged
QilongTang merged 5 commits intoDynamoDS:masterfrom
M-JULIANI:Wire-Icons-NewReferences
Sep 2, 2021
Merged

(Updated) Wires: New icon resources added.#12006
QilongTang merged 5 commits intoDynamoDS:masterfrom
M-JULIANI:Wire-Icons-NewReferences

Conversation

@M-JULIANI
Copy link

@M-JULIANI M-JULIANI commented Sep 2, 2021 •
edited
Loading

Description

This PR adds new icon references (higher resolution) of pins and watch icons to the DynamoCoreWpf project.

image

(Originally this PR was merging into DynamoNodeRedesign ref #11999, but has been modified to merge to master.)

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

Reviewers

@QilongTang

FYIs

@SHKnudsen
@Amoursol

QilongTang
QilongTang reviewed Sep 2, 2021
View reviewed changes
QilongTang
QilongTang reviewed Sep 2, 2021
View reviewed changes
Copy link
Contributor

@QilongTang QilongTang left a comment

Choose a reason for hiding this comment

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

One comment then LGTM

QilongTang
QilongTang reviewed Sep 2, 2021
View reviewed changes
QilongTang
QilongTang approved these changes Sep 2, 2021
View reviewed changes
Copy link
Contributor

@QilongTang QilongTang left a comment

Choose a reason for hiding this comment

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

One last comment, then LGTM

@QilongTang
Copy link
Contributor

QilongTang commented Sep 2, 2021

Thanks @M-JULIANI for addressing all the comments

@M-JULIANI
Copy link
Author

M-JULIANI commented Sep 2, 2021

@QilongTang are there other comments? :)

@QilongTang
Copy link
Contributor

QilongTang commented Sep 2, 2021

@QilongTang are there other comments? :)

Nope, only waiting for all the PR checks to finish. The Dynamo SelfServe would be the most important because that one check all the unit tests.

@M-JULIANI
Copy link
Author

M-JULIANI commented Sep 2, 2021 via email

@QilongTang
Copy link
Contributor

QilongTang commented Sep 2, 2021

Makes sense I guess merging to master will be quite a bit slower.
…
Sent from my iPhone
On Sep 2, 2021, at 3:24 PM, Aaron (Qilong) @.***> wrote:  @QilongTang are there other comments? :) Nope, only waiting for all the PR checks to finish. The Dynamo SelfServe would be the most important because that one check all the unit tests. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

Yes, because all unit tests passing required to produce Dynamo Daily builds

@QilongTang
Copy link
Contributor

QilongTang commented Sep 2, 2021 •
edited
Loading

@M-JULIANI @OliverEGreen @SHKnudsen You can see self serve passed on this PR, so I can merge now

@QilongTang QilongTang merged commit d0111ad into DynamoDS:master Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@QilongTang QilongTang QilongTang approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@M-JULIANI @QilongTang