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

Remove unsupported option from the app.src file #101

Merged
merged 1 commit into from
Apr 14, 2013
Merged

Remove unsupported option from the app.src file #101

merged 1 commit into from
Apr 14, 2013

Conversation

amiramix
Copy link

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
Copy link
Owner

eproxus commented Mar 25, 2013

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
Copy link
Author

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
Copy link
Contributor

horkhe commented Mar 26, 2013

@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
Copy link
Author

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
Copy link
Owner

eproxus commented Mar 31, 2013

@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
Copy link
Author

amiramix commented Apr 2, 2013

@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 added a commit that referenced this pull request Apr 14, 2013
Remove unsupported option from the app.src file
@eproxus eproxus merged commit 161e240 into eproxus:master Apr 14, 2013
@eproxus
Copy link
Owner

eproxus commented Apr 14, 2013

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

@amiramix
Copy link
Author

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

@eproxus eproxus added the bug label Jun 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants