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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Custom Script Engine #185

Merged
merged 44 commits into from May 18, 2019

Conversation

@BalestraPatrick
Copy link
Contributor

BalestraPatrick commented May 16, 2019

I continued working on #148 and fixed the conflicts with master that arose from the latest release, so I am creating a new PR for an easier review directly with master. Let me know if you prefer otherwise and I have another branch that doesn't have the conflicts fixed yet, so I could make a PR to feature/engines first and then to master. 馃憤

I did some extensive testing by even prototyping a simply NodeJS server and a script that does the communication part taking in list, upload and download commands and it's working as expected.

I also threw in some random typos and documentation fixes noticed in reading the existing code.
I didn't run Brittany yet, so I am sure there will be a ton of fixes done to my bad Haskell 馃槃

Regarding integration tests, I wrote engine.sh that quickly simulates an engine that simply stores the artifacts in a local folder. Added a bunch of integration tests covering all cases:

  • current-framework-engine-yml.bats
  • current-framework-named-engine-yml.bats
  • dynamic-frameworks-engine-yml.bats
  • static-frameworks-engine-yml.bats

I also renamed current-framework-yaml.bats to current-framework-yml.bats and current-framework-named-yaml.bats to current-framework-named-yml.bats for consistency.

Fixes #146

tmspzz and others added 30 commits Mar 11, 2019
src/Lib.hs Show resolved Hide resolved
Repository owner deleted a comment from hlintBot May 17, 2019
src/Lib.hs Show resolved Hide resolved
Repository owner deleted a comment from hlintBot May 17, 2019
@BalestraPatrick

This comment has been minimized.

Copy link
Contributor Author

BalestraPatrick commented May 18, 2019

@blender PR is now green since I fixed the integration the tests and a crash with INI configurations in cd1eaa5. I added some information to the README in 9c453c9 since I wanted to document how it works, we can always adjust later when a new version is cut.

Repository owner deleted a comment from hlintBot May 18, 2019
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
BalestraPatrick and others added 2 commits May 18, 2019
Co-Authored-By: Tommaso Piazza <196761+blender@users.noreply.github.com>
@tmspzz
tmspzz approved these changes May 18, 2019
Repository owner deleted a comment from hlintBot May 18, 2019
@tmspzz

This comment has been minimized.

Copy link
Owner

tmspzz commented May 18, 2019

PR looks good to me!
Excellent job and amazing contribution! 馃帀

@tmspzz tmspzz merged commit 3279870 into tmspzz:master May 18, 2019
1 check was pending
1 check was pending
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@tmspzz tmspzz deleted the BalestraPatrick:feature/engines-complete branch May 18, 2019
Repository owner deleted a comment from hlintBot May 18, 2019
@thii

This comment has been minimized.

Copy link
Contributor

thii commented May 31, 2019

@blender Could you release this to CocoaPods?

@tmspzz

This comment has been minimized.

Copy link
Owner

tmspzz commented May 31, 2019

Sure! @thii there is a pre-release version attached to the releases. Did you have a chance to try it?

@thii

This comment has been minimized.

Copy link
Contributor

thii commented May 31, 2019

I've tried it and found no problems so far.

@tmspzz

This comment has been minimized.

Copy link
Owner

tmspzz commented May 31, 2019

@thii it's out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can鈥檛 perform that action at this time.