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

Replace template<class> with template<typename> #260

Closed
wants to merge 5 commits into from
Closed

Replace template<class> with template<typename> #260

wants to merge 5 commits into from

Conversation

matssson
Copy link

Since the book focuses heavily on types, I think that the C++ keyword typename is more appropriate, clear and avoids confusion for those less experienced with C++, and I think it's generally good practice to use typename instead of the overloaded meaning of class.

I also added a missing step in the composition argument for fmap Nothing and fixed some whitespace formatting.
There is trailing whitespace in the licence file and in errata-1.0.0.md, but I didn't change them since I think it's more interesting for the readers to know when they were last updated properly.

@@ -206,6 +206,8 @@ \subsection{Equational Reasoning}
fmap g Nothing
= { definition of fmap }
fmap g (fmap f Nothing)
= { definition of composition }
Copy link
Collaborator

Choose a reason for hiding this comment

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

This snippet seems to be out of scope of this PR, is it intended ?

Copy link
Author

Choose a reason for hiding this comment

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

It was intended, but you're right that it's out of scope - I'll remove it.

Also thanks for the reminder, I completely forgot about this PR. I'll fix it this week.

Copy link
Collaborator

Choose a reason for hiding this comment

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

no problem :)

@drupol
Copy link
Collaborator

drupol commented Feb 2, 2023

Dear @matssson,

Do you plan to implement the changes in this PR ?

Thanks

@matssson
Copy link
Author

matssson commented Feb 12, 2023

I fixed the changes now in a new PR #309

I redid the trailing whitespace part since there were a lot of changes since the original PR.

@drupol drupol closed this Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants