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

fixes error on documentation for filterMap #569

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@rebelwarrior
Contributor

rebelwarrior commented Apr 27, 2016

This line filterMap String.toInt ["3", "4.0", "5", "hats"] == [3,5] would not work since toInt doesn't produce a Maybe but a Result.

fixes error on doc for filterMap
This line `filterMap String.toInt ["3", "4.0", "5", "hats"] == [3,5]` would not work since toInt doesn't produce a Maybe but a Result.
@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Apr 27, 2016

Contributor

Two comments:

  1. Don't do the List. prefix for filterMap there. I think this is done nowhere in the documentation, that documentation for function f is prefixed by its module name inside that defining module.
  2. A similar fix in the documentation for Signal.filterMap should probably be done in the same go.
Contributor

jvoigtlaender commented Apr 27, 2016

Two comments:

  1. Don't do the List. prefix for filterMap there. I think this is done nowhere in the documentation, that documentation for function f is prefixed by its module name inside that defining module.
  2. A similar fix in the documentation for Signal.filterMap should probably be done in the same go.
@rebelwarrior

This comment has been minimized.

Show comment
Hide comment
@rebelwarrior

rebelwarrior Apr 27, 2016

Contributor

Oh shoot I thought I took the prefix out. That was an oversight. (Copy paste from command line)

I'll do another pull request tonight.

Sent from my mobile.

On Apr 27, 2016, at 2:47 AM, Janis Voigtländer notifications@github.com wrote:

Two comments:

Don't do the List. prefix for filterMap there. I think this is done nowhere in the documentation, that documentation for function f is prefixed by its module name inside that defining module.
A similar fix in the documentation for Signal.filterMap should probably be done in the same go.

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

Contributor

rebelwarrior commented Apr 27, 2016

Oh shoot I thought I took the prefix out. That was an oversight. (Copy paste from command line)

I'll do another pull request tonight.

Sent from my mobile.

On Apr 27, 2016, at 2:47 AM, Janis Voigtländer notifications@github.com wrote:

Two comments:

Don't do the List. prefix for filterMap there. I think this is done nowhere in the documentation, that documentation for function f is prefixed by its module name inside that defining module.
A similar fix in the documentation for Signal.filterMap should probably be done in the same go.

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Apr 27, 2016

Contributor

Cool. Actually, you can keep it simple, I think. Just remove the prefix and squash the commits. I just realized that making a corresponding change to the documentation of Signal.filterMap is not really necessary, with the changes to core that are on the horizon.

Contributor

jvoigtlaender commented Apr 27, 2016

Cool. Actually, you can keep it simple, I think. Just remove the prefix and squash the commits. I just realized that making a corresponding change to the documentation of Signal.filterMap is not really necessary, with the changes to core that are on the horizon.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Apr 27, 2016

Contributor

Thanks for opening the other PR. Closing this one here in favour of the other one.

Contributor

jvoigtlaender commented Apr 27, 2016

Thanks for opening the other PR. Closing this one here in favour of the other one.

@rebelwarrior

This comment has been minimized.

Show comment
Hide comment
@rebelwarrior

rebelwarrior Apr 27, 2016

Contributor

I could not find it to close it. Thought you'd closed it already. :)

Sent from my mobile.

On Apr 27, 2016, at 1:57 PM, Janis Voigtländer notifications@github.com wrote:

Thanks for opening the other PR. Closing this one here in favour of the other one.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

Contributor

rebelwarrior commented Apr 27, 2016

I could not find it to close it. Thought you'd closed it already. :)

Sent from my mobile.

On Apr 27, 2016, at 1:57 PM, Janis Voigtländer notifications@github.com wrote:

Thanks for opening the other PR. Closing this one here in favour of the other one.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment