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

Change thrown exception type in Window's class constructor #347

Merged
merged 6 commits into from
Oct 19, 2023

Conversation

ezioleq
Copy link
Contributor

@ezioleq ezioleq commented Oct 19, 2023

Throw PlatformNotSupportedException instead of the InvalidOperationException in the Window's class constructor as was said in #242

Throw PlatformNotSupportedException instead of the InvalidOperationException in Window's constructor
kkard2
kkard2 previously approved these changes Oct 19, 2023
Copy link
Contributor

@kkard2 kkard2 left a comment

Choose a reason for hiding this comment

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

unreal (i don't remember how it was supposed to be resolved, wait for @Vixenka to lgtm this)

Copy link
Member

@Vixenka Vixenka left a comment

Choose a reason for hiding this comment

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

Generally thank you for the contributing!

But you do not understand issue well, because you change this on the managed side, but you should change this on this line:

InvalidOperationError::with_str("Window is not supported on this device.").into(),

But it still fine, repair this and retry, also remember about C# code style, because you use another :)

@kkard2 kkard2 dismissed their stale review October 19, 2023 19:03

average vi response:

@ezioleq ezioleq requested a review from Vixenka October 19, 2023 19:25
Copy link
Member

@Vixenka Vixenka left a comment

Choose a reason for hiding this comment

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

Fantastic! You done this! LGTM!

@Vixenka Vixenka merged commit 7a96cb8 into NoiseStudio:master Oct 19, 2023
1 check passed
@ezioleq ezioleq deleted the NE-242 branch October 19, 2023 20:23
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.

None yet

3 participants