| Author |
Message |
< Erlang patches mailing list ~ Fix searching for source files in cover:analyse_to_file/* |
| Guest |
 Posted: Sat Apr 24, 2010 7:27 am |
 |
|
|
Guest
|
On Fri, Apr 23, 2010 at 10:56 PM, Jeremy Raymond <jeraymond@gmail.com> wrote:
> I've patched cover to search for source files in the location referenced in
> the .beam file instead of just in the .beam directory and in the ../src
> relative dir. This is based upon a patch by Thomas Arts posted to
> erlang-questions several years ago that never made it into otp (
> http://www.erlang.org/cgi-bin/ezmlm-cgi/4/29048).
>
> git fetch git://github.com/jeraymond/otp.git cover_src_path_fix
Thanks!
Your patch removes the export of three documented functions. Why?
The code itself is easy to follow and looks reasonable (and the test
suite has been updated!), but the spacing is inconsistent both in the
code itself and compared to the rest of the module. There should a
space after commas in function calls, and spaces around list
comprehension generators (" <- "), but list matching
should be written without spaces like "[Main|_]".
Also, "_" variables in the last clause of a case should be avoided
if the value is known. So it should be:
case filelib:is_file(Main) of
true ->
...;
false ->
...
end.
and so on for all the other "_" variables.
I would also recommend that path_in_beam simply returns
'error' instead of an error tuple whose value is never used.
--
Björn Gustavsson, Erlang/OTP, Ericsson AB
________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org
Post received from mailinglist |
|
|
| Back to top |
|
| Guest |
| Posted: Sat Apr 24, 2010 7:27 am |
|
|
|
Guest
|
On Fri, Apr 23, 2010 at 10:56 PM, Jeremy Raymond <jeraymond@gmail.com> wrote:
> I've patched cover to search for source files in the location referenced in
> the .beam file instead of just in the .beam directory and in the ../src
> relative dir. This is based upon a patch by Thomas Arts posted to
> erlang-questions several years ago that never made it into otp (
> http://www.erlang.org/cgi-bin/ezmlm-cgi/4/29048).
>
> git fetch git://github.com/jeraymond/otp.git cover_src_path_fix
Thanks!
Your patch removes the export of three documented functions. Why?
The code itself is easy to follow and looks reasonable (and the test
suite has been updated!), but the spacing is inconsistent both in the
code itself and compared to the rest of the module. There should a
space after commas in function calls, and spaces around list
comprehension generators (" <- "), but list matching
should be written without spaces like "[Main|_]".
Also, "_" variables in the last clause of a case should be avoided
if the value is known. So it should be:
case filelib:is_file(Main) of
true ->
...;
false ->
...
end.
and so on for all the other "_" variables.
I would also recommend that path_in_beam simply returns
'error' instead of an error tuple whose value is never used.
--
Björn Gustavsson, Erlang/OTP, Ericsson AB
________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org
Post received from mailinglist |
|
|
| Back to top |
|
| Guest |
| Posted: Sat Apr 24, 2010 1:24 pm |
|
|
|
Guest
|
Hello,
My mistake with the exports, I'd seen it as the functions being exported
twice but really it was different spellings of the functions: analyse with
an S vs analyze with a Z. It looks like analyse with the S are the
documented functions... however I better restore it, someone must be using
the Z versions.
I'll clean up the code as per you suggestions below and post an update.
--
Jeremy Raymond
2010/4/24 Bj |
|
|
| Back to top |
|
| Guest |
| Posted: Sat Apr 24, 2010 1:25 pm |
|
|
|
Guest
|
Hello,
My mistake with the exports, I'd seen it as the functions being exported
twice but really it was different spellings of the functions: analyse with
an S vs analyze with a Z. It looks like analyse with the S are the
documented functions... however I better restore it, someone must be using
the Z versions.
I'll clean up the code as per you suggestions below and post an update.
--
Jeremy Raymond
2010/4/24 Bj |
|
|
| Back to top |
|
| Guest |
| Posted: Sat Apr 24, 2010 7:24 pm |
|
|
|
Guest
|
I've made the suggested changes. Please take a look and let me know if there
is anything else. Thanks!
git fetch git://github.com/jeraymond/otp.git cover_src_path_fix
--
Jeremy Raymond
2010/4/24 Jeremy Raymond <jeraymond@gmail.com>
> Hello,
>
> My mistake with the exports, I'd seen it as the functions being exported
> twice but really it was different spellings of the functions: analyse with
> an S vs analyze with a Z. It looks like analyse with the S are the
> documented functions... however I better restore it, someone must be using
> the Z versions.
>
> I'll clean up the code as per you suggestions below and post an update.
>
> --
> Jeremy Raymond
>
>
> 2010/4/24 Bj |
|
|
| Back to top |
|
| Guest |
| Posted: Sat Apr 24, 2010 7:25 pm |
|
|
|
Guest
|
I've made the suggested changes. Please take a look and let me know if there
is anything else. Thanks!
git fetch git://github.com/jeraymond/otp.git cover_src_path_fix
--
Jeremy Raymond
2010/4/24 Jeremy Raymond <jeraymond@gmail.com>
> Hello,
>
> My mistake with the exports, I'd seen it as the functions being exported
> twice but really it was different spellings of the functions: analyse with
> an S vs analyze with a Z. It looks like analyse with the S are the
> documented functions... however I better restore it, someone must be using
> the Z versions.
>
> I'll clean up the code as per you suggestions below and post an update.
>
> --
> Jeremy Raymond
>
>
> 2010/4/24 Bj |
|
|
| Back to top |
|
| Guest |
| Posted: Mon Apr 26, 2010 11:25 am |
|
|
|
Guest
|
2010/4/24 Jeremy Raymond <jeraymond@gmail.com>:
> I've made the suggested changes. Please take a look and let me know if there
> is anything else. Thanks!
>
> git fetch git://github.com/jeraymond/otp.git cover_src_path_fix
Thanks! Looks much better. I will include it in 'pu' with only a minor
change - I will replace
filename:join([Dir,Path])
with
filename:join(Dir, Path)
--
Björn Gustavsson, Erlang/OTP, Ericsson AB
________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org
Post received from mailinglist |
|
|
| Back to top |
|
| Guest |
| Posted: Mon Apr 26, 2010 11:25 am |
|
|
|
Guest
|
2010/4/24 Jeremy Raymond <jeraymond@gmail.com>:
> I've made the suggested changes. Please take a look and let me know if there
> is anything else. Thanks!
>
> git fetch git://github.com/jeraymond/otp.git cover_src_path_fix
Thanks! Looks much better. I will include it in 'pu' with only a minor
change - I will replace
filename:join([Dir,Path])
with
filename:join(Dir, Path)
--
Björn Gustavsson, Erlang/OTP, Ericsson AB
________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org
Post received from mailinglist |
|
|
| Back to top |
|
| Guest |
| Posted: Mon Apr 26, 2010 12:01 pm |
|
|
|
Guest
|
|
| Back to top |
|
| Guest |
| Posted: Mon Apr 26, 2010 12:01 pm |
|
|
|
Guest
|
|
| Back to top |
|
|
|
Digg It
Del.icio.us
Reddit
Facebook
Stumble Upon
Technorati
