Skip to content

feat: Requests and Events#37

Merged
blorp-goes-the-derp merged 19 commits intoobs-websocket-community-projects:developfrom
ChristopheCVB:feat/requests_and_events
May 21, 2021
Merged

feat: Requests and Events#37
blorp-goes-the-derp merged 19 commits intoobs-websocket-community-projects:developfrom
ChristopheCVB:feat/requests_and_events

Conversation

@ChristopheCVB
Copy link
Copy Markdown
Member

@ChristopheCVB ChristopheCVB commented May 19, 2021

Upgraded to Gradle 6.8.3
Upgraded Gson dependency to latest version
A bit of Cleanup
Added some Requests and Events
Refactor Events

@blorp-goes-the-derp
Copy link
Copy Markdown
Collaborator

Thanks! Taking a look...

Copy link
Copy Markdown
Collaborator

@blorp-goes-the-derp blorp-goes-the-derp left a comment

Choose a reason for hiding this comment

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

I've reviewed the files and will try testing locally next...Only request so far was some more javadocs on one place, otherwise looks fantastic so far!

Comment thread src/main/java/net/twasi/obsremotejava/objects/Scene.java Outdated
Comment thread src/main/java/net/twasi/obsremotejava/OBSCommunicator.java Outdated
Comment thread src/main/java/net/twasi/obsremotejava/OBSRemoteController.java Outdated
Copy link
Copy Markdown
Collaborator

@blorp-goes-the-derp blorp-goes-the-derp left a comment

Choose a reason for hiding this comment

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

A bit more review, requesting some changes on one thing so far.
Otherwise testing was good, and I'll post a message to the original maintainers asking for their final feedback / if we can merge after the requested changes.

Comment thread src/main/java/net/twasi/obsremotejava/OBSRemoteController.java Outdated
Comment thread src/main/java/net/twasi/obsremotejava/OBSRemoteController.java Outdated
Comment thread src/main/java/net/twasi/obsremotejava/OBSRemoteController.java Outdated
Comment thread src/main/java/net/twasi/obsremotejava/OBSRemoteController.java Outdated
Copy link
Copy Markdown
Collaborator

@blorp-goes-the-derp blorp-goes-the-derp left a comment

Choose a reason for hiding this comment

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

Looks great! I approve!
Still pending response from owners, I'll ping them and if there's no response in a few days then merge if that's fine with you.

@Laraakaa Laraakaa self-requested a review May 20, 2021 19:27
Copy link
Copy Markdown
Contributor

@Laraakaa Laraakaa left a comment

Choose a reason for hiding this comment

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

Thanks for this awesome extension! Looks good to me, we'll make sure to get this released once it's merged as new minor version!

@ChristopheCVB
Copy link
Copy Markdown
Member Author

Thanks for this awesome extension! Looks good to me, we'll make sure to get this released once it's merged as new minor version!

Thanks for the kind words.
Fine by me ;)

@ChristopheCVB
Copy link
Copy Markdown
Member Author

I'm sorry guys, there were things that bothered me. I went ahead and refactored a bit.
No change to the public API as long as developers use lambdas.

Comment thread build.gradle
@blorp-goes-the-derp
Copy link
Copy Markdown
Collaborator

Ran the tests again, all seems well.

@ChristopheCVB
Copy link
Copy Markdown
Member Author

Ran the tests again, all seems well.

I added some too and ran them locally before pushing each commit, including the OBSRemoteControllerUnsecuredIT

@blorp-goes-the-derp blorp-goes-the-derp merged commit a29b210 into obs-websocket-community-projects:develop May 21, 2021
@ChristopheCVB ChristopheCVB deleted the feat/requests_and_events branch May 21, 2021 16:26
@blorp-goes-the-derp blorp-goes-the-derp mentioned this pull request Jun 13, 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