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.

Sign up for GitHub

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

Jump to bottom

Make it so that the max_function_length style checker can ignore func… #62

Merged
merged 1 commit into from
Oct 6, 2016
Merged

Make it so that the max_function_length style checker can ignore func… #62

merged 1 commit into from
Oct 6, 2016

Conversation

sargun
Copy link
Contributor

@sargun sargun commented Oct 5, 2016

…tions

@gadgetci
Copy link
Member

gadgetci commented Oct 5, 2016

xref failed with exit status: 1

gadgetci
gadgetci reviewed Oct 5, 2016
View reviewed changes
src/elvis_style.erl
@@ -517,22 +517,25 @@ max_module_length(Config, Target, RuleConfig) ->
[]
end.

-type max_function_length_config() :: #{ignore_functions => [function_spec()]
max_length => non_neg_integer()}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to Xref:

syntax error before: max_length

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing a comma here

gadgetci
gadgetci reviewed Oct 5, 2016
View reviewed changes
src/elvis_style.erl
-spec max_function_length(elvis_config:config(),
elvis_file:file(),
empty_rule_config()) ->
max_function_length_config()) ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to Xref:

type max_function_length_config() undefined

elbrujohalcon
elbrujohalcon requested changes Oct 5, 2016
View reviewed changes
Copy link
Member

@elbrujohalcon elbrujohalcon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, @sargun! Would you mind adding tests for this?

src/elvis_style.erl
@@ -517,22 +517,25 @@ max_module_length(Config, Target, RuleConfig) ->
[]
end.

-type max_function_length_config() :: #{ignore_functions => [function_spec()]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To preserve module style, you should define this type above, close to the other types defined in the module

@sargun
Copy link
Contributor Author

sargun commented Oct 6, 2016

Any idea why common test is throwing the following:

3c075477e55e:elvis_core sdhillon$ rebar3 ct
===> Verifying dependencies...
===> Compiling elvis
===> Failed to restore /Users/sdhillon/repos/lashup/_checkouts/elvis_core/_build/test/lib/elvis/.rebar3/erlcinfo file. Discarding it.

===> Compiling _build/test/lib/elvis/test/examples/fail_macro_module_names.erl failed
_build/test/lib/elvis/test/examples/fail_macro_module_names.erl:15: redefining predefined macro 'FUNCTION_NAME'

_build/test/lib/elvis/test/examples/fail_macro_module_names.erl:21: Warning: missing specification for function module_name/0
_build/test/lib/elvis/test/examples/fail_macro_module_names.erl:25: Warning: missing specification for function function_name/0
_build/test/lib/elvis/test/examples/fail_macro_module_names.erl:30: Warning: missing specification for function build_binary/0
_build/test/lib/elvis/test/examples/fail_macro_module_names.erl:38: Warning: missing specification for function no_errors/0

@sargun
Copy link
Contributor Author

sargun commented Oct 6, 2016

Fixed.

@elbrujohalcon elbrujohalcon merged commit 387f664 into inaka:master Oct 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gadgetci gadgetci gadgetci left review comments

@elbrujohalcon elbrujohalcon elbrujohalcon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sargun @gadgetci @elbrujohalcon