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

Add default value to Spy constructor argument #156

Merged
merged 2 commits into from
Dec 13, 2018

Conversation

JeroenDeDauw
Copy link
Contributor

This allows replacing

mySpy = spy.new(function() end)

with

mySpy = spy.new()

This allows replacing

```lua
mySpy = spy.new(function() end)
```

with

```lua
mySpy = spy.new()
```
@JeroenDeDauw
Copy link
Contributor Author

Submitting without tests as I do not know if this change is welcome. If is it, I will happily add some tests to this PR.

@ajacksified
Copy link
Contributor

Seems reasonable. Add some tests and 👍

@JeroenDeDauw
Copy link
Contributor Author

Note that I did not run this test as I do not have lua installed

@JeroenDeDauw
Copy link
Contributor Author

Would be nice to have some instructions on how to run the tests locally, especially without installing lua. In my project I added some docker stuff that allows me to run

docker-compose run --rm app busted

That won't work here since it is busted itself, and it is not really clear to me how to make it work. Seems like it gets rather involved with all the stuff in .travis/.

@Tieske
Copy link
Member

Tieske commented Oct 28, 2018

@ajacksified just tested it locally. lgtm.

@Tieske
Copy link
Member

Tieske commented Dec 2, 2018

@ajacksified can we merge this?

@ajacksified ajacksified merged commit 3b2351c into lunarmodules:master Dec 13, 2018
@JeroenDeDauw JeroenDeDauw deleted the patch-1 branch December 14, 2018 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants