Skip to content

Conversation

@tequdev
Copy link
Contributor

@tequdev tequdev commented Aug 1, 2024

High Level Overview of Change

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release

Before / After

Test Plan

@@ -1,5 +1,5 @@
const Hook = (arg) => {
return accept('', 0)
return accept('base: Finished.', 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

expect(hookExecutions.executions[0].HookReturnString).toMatch(
'base: Finished.'
)

@dangell7
Copy link
Member

dangell7 commented Aug 2, 2024

Those tests are actually going to be removed. They are for testing the c++ code. If they compile its not a big deal but the c++ does not have access to the jshooks-api so imo these need to be standalone.

The other changes I do like though.

@tequdev
Copy link
Contributor Author

tequdev commented Aug 4, 2024

Those tests are actually going to be removed. They are for testing the c++ code. If they compile its not a big deal but the c++ does not have access to the jshooks-api so imo these need to be standalone.

The other changes I do like though.

If they are finally removed, that's fine.

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.

2 participants