-
Notifications
You must be signed in to change notification settings - Fork 518
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
reset hooks under profiles for application opts #1564
Conversation
src/rebar_app_discover.erl
Outdated
not lists:member(Key, [post_hooks, | ||
pre_hooks, | ||
provider_hooks, | ||
artifacts])]}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd extract that list of magic keys to escape here and on line 188 to some shared variable or macro so they always end up matching.
test/rebar_hooks_SUITE.erl
Outdated
@@ -98,6 +100,17 @@ run_hooks_once(Config) -> | |||
rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]), | |||
rebar_test_utils:run_and_check(Config, RebarConfig, ["compile"], {ok, [{app, Name, valid}]}). | |||
|
|||
%% test that even if a hook is defined in a used profile it is not run twice |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The discussion on IRC mentioned things that looked like umbrella apps, but here it doesn't seem to matter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test does create an umbrella app, just like the one above it, just with the extra piece that it uses a profile. I can add that fact to the comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would make it clearer :)
f724cf1
to
18628fd
Compare
18628fd
to
48dc973
Compare
No description provided.