search

eproxus / meck

A mocking library for Erlang
http://eproxus.github.io/meck
Apache License 2.0
813 stars 231 forks source link

Remove unsupported option from the app.src file #101

Closed amiramix closed 11 years ago

amiramix commented 11 years ago

Key build_dependencies currently specified in the .app files isn't recognized by Erlang release tools (systools and reltool). This is a non-standard key that is unsupported by those tools, causing warnings every time the .app files is processed by those tools (which happens when creating releases or calling most of the functions available in either systools or reltool modules). It seems that the key is not used anyway so I think it should be safe to remove it, but if indeed it serves an unknown to me purpose please let me know and I will be happy to update the code as well.

The list of keys supported by the release tools is available here: http://www.erlang.org/doc/design_principles/applications.html#id73724

eproxus commented 11 years ago

Thanks for spotting this, it's just an old legacy from when the app file was created 3 years ago. :-)

Could you repoint the pull request to the branch develop which is where active development is going on?

amiramix commented 11 years ago

Actually this line has already been removed in the master branch (three months ago). When do you plan to integrated develop to master? Is it safe to use develop?

horkhe commented 11 years ago

@amiramix the develop branch has decent 88% unit test coverage so you can say that it is quite safe. Besides being a test tool Meck has an advantage that even if it has a bug you won't break anything in production since it does not run there :).

At my company we test using the develop branch because it has features that we need and that are not yet available in the master branch. That was the reason why I started contributing to the project in the first place. If you are interested you may read about them here.

I would even encourage you to use the develop branch and if you do find a bug, which is unlikely but possible, then please report it here and we will fix it.

amiramix commented 11 years ago

I see, it's actually Zotonic that is using it, not me, and I can't change it from master to develop that easily. Do you know when develop may be integrated to master? Otherwise I may try to ask them to switch to develop. Thanks

eproxus commented 11 years ago

@amiramix In theory, develop can be integrated to master right now. However, there are a few outstanding pull requests and some tickets that we need to look at first. In that sense it is safe to work with develop, since the intention is always to be backwards compatible and stable enough for production use (tests, that is).

Since this is a new change, I'd prefer to merge it in to develop first, and then master. If you need this on master, just let me know after I've merged the updated pull request (you pointing it to develop) and I'll cherry pick it into master.

@horkhe The coverage is actually 93% (check .eunit/index.html). It's just Rebar that reports it a bit funnily in the shell (see https://github.com/basho/rebar/issues/354)

amiramix commented 11 years ago

@eproxus thanks for the info. This pull request is already pointing to master. As I mentioned earlier, that line has already been removed in develop so I can't re-point it because there would be no change to integrate. It's not urgent so I think I will wait. If it turns out to be too long I will try to temporarily switch to develop.

eproxus commented 11 years ago

Merged. Please let me know if you need anything else to use meck as a dependency.

amiramix commented 11 years ago

OK, great, thanks! I will let you know in case I encounter other problems.