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

Improvements to chapter 8 (Reuse and composition at work) #57

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

koendehondt
Copy link
Collaborator

This is the first-pass review of chapter 8 "Reuse and composition at work".

There are still issues to tackle in future PRs:

  • I think adding numbered labels to the different parts in the first figure would be helpful as a reference for the reader.
  • The caption of Figure 8-5 reads: "Live coding your widgets (no it is not openWithSpec but open my friend but the background is supercool!". I understand the super coolness of the photo with the lighthouse in the background, but the code in the photo is wrong and the caption is not very professional. I think we should replace the figure and adapt the caption.
  • In section 8.7 "Managing three widgets and their interactions", messages like add: #symbolHere are used, but I think that API had not been explained before.
  • In section 8.10 "Changing the layout of a reused widget", the code just above "SHOULD Update" does not handle the case when there is no selection. Shouldn't we add that? Or does that complicate the example?
  • Fix "SHOULD Update" and fix "ESTEBAN how can I do" in section 8.11 "Changing layouts".
  • In section 8.11 "Changing layouts", there is a message "layoutAlternative1", but the method is not defined.
  • In general, we should decide to use "widget" or "presenter". In this PR, I changed some "widget" to "presenter", but not all. Will we use them interchangeably, or is it better to use "presenter" all the way?

@Ducasse
Copy link
Member

Ducasse commented Mar 28, 2024

The changes are good and your questions too.
about the note the latex template supports it but not microdow so I will extend microdown so that we have a simple tag to map the note to the latex macro.
For your questions left tell me what you prefer:
(1) we continue to the point we have several chapters, I print them all and I come to discuss with you about the action. Are you working 100%? Because I thought that you worked 80%.
Or we can do it during a sprint. So let me know.

@Ducasse Ducasse merged commit 042201b into master Apr 4, 2024
1 check passed
@koendehondt koendehondt deleted the kdh-review-chapter-8 branch April 4, 2024 19:04
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