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

feat: Use Rust FFI #210

Closed
wants to merge 1 commit into from
Closed

feat: Use Rust FFI #210

wants to merge 1 commit into from

Conversation

tienvx
Copy link
Contributor

@tienvx tienvx commented Aug 21, 2021

No description provided.

@tienvx tienvx changed the title Use Rust FFI feat: Use Rust FFI Aug 21, 2021
@tienvx tienvx force-pushed the rust-ffi branch 3 times, most recently from 8340058 to 1561833 Compare August 22, 2021 01:47
@tienvx
Copy link
Contributor Author

tienvx commented Aug 22, 2021

Something wrong with the CI:

  • It should only run on 7.4 and 8.0, but it also run on 7.3
  • Linter is warning about code on master, not code from this branch, because this issue is already fixed on this branch:
diff --git a/src/PhpPact/Consumer/Model/ConsumerRequest.php b/src/PhpPact/Consumer/Model/ConsumerRequest.php
index 9fe9d22..1e2732d 100644
--- a/src/PhpPact/Consumer/Model/ConsumerRequest.php
+++ b/src/PhpPact/Consumer/Model/ConsumerRequest.php
@@ -94,8 +94,8 @@ class ConsumerRequest implements \JsonSerializable
     }
 
     /**
-     * @param string         $header
-     * @param array | string $value
+     * @param string       $header
+     * @param array|string $value
      *
      * @return ConsumerRequest
      */
diff --git a/src/PhpPact/Consumer/Model/ProviderResponse.php b/src/PhpPact/Consumer/Model/ProviderResponse.php
index dd22f88..a9f9dee 100644
--- a/src/PhpPact/Consumer/Model/ProviderResponse.php
+++ b/src/PhpPact/Consumer/Model/ProviderResponse.php
@@ -64,8 +64,8 @@ class ProviderResponse implements \JsonSerializable
     }
 
     /**
-     * @param string         $header
-     * @param array | string $value
+     * @param string       $header
+     * @param array|string $value
      *
      * @return ProviderResponse
      */

@cfmack
Copy link
Collaborator

cfmack commented Aug 22, 2021

Wow, moving to Rust is huge! How can I assist as I see the CI is not passing.

@tienvx
Copy link
Contributor Author

tienvx commented Aug 22, 2021

Still problems in my comment above. I think CI is cached so it always run against master branch. I don't know how to fix from my side. Github Actions in my repository is still working fine https://github.com/tienvx/pact-php/actions

EDIT: I didn't know Github Actions has this workflow approval feature.

@tienvx tienvx force-pushed the rust-ffi branch 2 times, most recently from e9f948d to 16d8484 Compare August 23, 2021 16:33
@mefellows
Copy link
Member

(I've just hit approve @tienvx to run your build)

@tienvx
Copy link
Contributor Author

tienvx commented Nov 12, 2021

@cfmack I fixed the conflict, please take a look again

@tienvx tienvx force-pushed the rust-ffi branch 2 times, most recently from 7adc919 to aefbaba Compare November 20, 2021 12:26
@tienvx
Copy link
Contributor Author

tienvx commented Nov 20, 2021

Your merge commit has syntax error ce65785. I re-based and forced push anyway. New code support specification v4.0 . Tests will not be passed until version 0.1.3 of FFI library is released. But the code is ready to be reviewed.

Question: Do we need a new branch 8.x on this repository?

@tienvx
Copy link
Contributor Author

tienvx commented Dec 3, 2021

Updated Pact FFI library to 0.1.3 . Tests will be passed when approve.

@tienvx tienvx force-pushed the rust-ffi branch 3 times, most recently from e7f1568 to 02fcbec Compare December 13, 2021 23:39
@tienvx tienvx force-pushed the rust-ffi branch 3 times, most recently from eb47722 to 2dc1b6a Compare March 7, 2022 15:14
@tienvx
Copy link
Contributor Author

tienvx commented Mar 7, 2022

@cfmack In case you haven't seen this comment on Slack yet https://pact-foundation.slack.com/archives/C9W94PXPY/p1646321219144749

@tienvx
Copy link
Contributor Author

tienvx commented Jan 3, 2023

Closed as split into smaller pull requests.

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