-
Notifications
You must be signed in to change notification settings - Fork 63
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
Third review #562
Third review #562
Conversation
|
||
a. Chapters on their own a solid. I think there are times where I felt myself | ||
wanting a bit more "glue" or context. For example, as currently presented, | ||
learning objectives are separated from chapters. I want to know at the beginning |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good point. We include our key points in an appendix and at the end of each chapter, but our learning objectives are only included in an appendix. I'd support including the learning objectives at the beginning of each chapter too (immediately before or after the Terry Pratchett quote).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm 👎 on this:
-
Now that we have better opening sentences for chapters and sections, readers will know what the road ahead looks like.
-
It adds length to the print version (which is already quite long).
-
They're most useful to instructors teaching courses, for whom the consolidated version at the back is most useful.
-
It's December 1 and we need to ship this, so I'm 👎 on non-essential changes in general.
|
||
b. I wrote my answer to question 4 (comment a) before I finished the | ||
introduction to the first chapter. I feel like I am still not 100% convinced | ||
about the title. It works as is without being misleading, but I think revisions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already discussed the title in #529 (and decided to add a subtitle), so probably no need to revisit that issue.
revisions, such as additions or omissions? Please be as specific as possible. | ||
|
||
a. Looking at the overall order of chapters---would it be beneficial to reorder | ||
in a way that more closely resembles how an RSE might create a project. This |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We literally create a project in the book, so I'm not sure you can argue that the chapter order doesn't resemble how an RSE might create a project.
irreproducible or unsustainable, but "not against it" and actively supporting it | ||
are very different things. Academia doesn't yet know reward people for writing | ||
useful software, so while you may be thanked, the effort you put in may not | ||
translate into job security or decent pay." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The grammatical re-write has been done during subsequent proof reading: "Academia doesn’t yet know how to reward people..."
|
||
b. Sec 1.1: Mentions "three" related concepts; why not give sustainability its | ||
own bullet point? I love your definition. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Section 0.1 sustainability now has its own bullet point.
a. Looking at the overall order of chapters---would it be beneficial to reorder | ||
in a way that more closely resembles how an RSE might create a project. This | ||
might suggest for example starting with Git to get things organized and under | ||
version control right away. Does chapter 7 on working with teams belong earlier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with where version control appears in the chapter order.
I agree that the teams chapter could kind of go anywhere, but we've discussed it at length and I'm comfortable with where we've put it.
|
||
e. Sec 1.2.1: With the exception of your clearly stating Python 3 is needed, | ||
might it be better to specify versions that were used in the creation of the | ||
book examples? I think it is fine to allow learners freedom, especially since at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A call-out box in the software installation section mentioning what version of git, python, bash and make we/Amira used when writing the book might be useful, just to alert the reader to the fact that if their output looks slightly different than ours, the difference might be due to different versions.
In the provenance chapter we talk about archiving the conda environment.yml
file to document a software environment, so we could also archive Amira's...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm 👍 on a brief list in the software installation section of the versions we're using at the time of writing. I'm 👎 on adding more to the provenance chapter (see comments above about including learning objectives in each chapter, non-essential changes, and it being December 1).
for patterns in data is common to most scientific analyses and so you have | ||
chosen a straightforward example that will allow you to teach the concrete | ||
objectives you state. Rather than Syllabus, maybe something more direct---"What | ||
you will learn." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like "What You Will Learn" as an alternative section heading to "Syllabus"
intermediate level. I'd like to believe that this level of user at least knows | ||
how to manipulate Linux directories. Everything in this chapter felt like | ||
prerequisite, so perhaps it should be an appendix? | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to agree that for most people in our target audience, the Basics of the Unix Shell chapter is probably mostly stuff they already know (and three chapters on the Unix Shell does seem like a lot). Having said that:
- In my experience many people have trouble with relative vs absolute paths
- It can be nice to start with content that you're mostly familiar with to build confidence before moving into harder topics
I'm therefore probably happy to leave the Basics of the Unix Shell chapter as is, unless others have a strong opinion about moving it to an appendix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's totally feasible to be an intermediate R programmer without ever having to really interact with the shell, is this different for Python users?
We don't really introduce anything critical to the project in this chapter, except maybe an example of using wc
, so it could be moved. But, like @DamienIrving, I don't have strong feelings about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my grad school experience, there seems to exist many researchers who know there way around Python or R, but tend to avoid the shell as much as possible. I don't think there is any harm in asking more advanced shell users to skip the shell basics chapter, but I would also be onboard with putting in the appendix and ask the novice shell users to read it before continuing in the book.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a strong 👎 on major surgery at this point. No matter what we ship, someone will have been better served by some better selection or arrangement of topics. What we have now is coherent and useful - if it turns out we were badly wrong, we can ship Version 2 after the R book comes out.
bug reports. | ||
|
||
j. I think Docker is an essential reproducibility tool and feel that a chapter | ||
or section on the subject would be useful. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a call-out box about Docker in the provenance chapter, with a citation to more information. I think that's probably sufficient.
important for RSEs to be effective in that role. | ||
|
||
i. Should Sec 7.6 discuss using GitHub issues templates to create more effective | ||
bug reports. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitHub issues templates are now discussed in Section 8.6 (Bug Reports).
k. The Finale at this point seems like an afterthought. Perhaps the great | ||
content on working within teams and communities could be reshaped into something | ||
that sends people off thinking about (and hopefully acting on) the helpful | ||
advice presented in these sections? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hate to say it because we had a go at improving the finale in #522, but I still think it misses the mark and reads like an afterthought. We decided earlier on that moving the teams content to the end was not the answer (we didn't want it to seem like an afterthought), so what is it we'd like to say to people at the end of a course like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some ideas:
- We could talk about how we don't imagine our readers implementing all of these ideas at once, but hopefully they'll be able to recognize when a particular tool will be useful.
- We could talk about things we haven't covered that might be next steps.
- We could welcome the reader to the research software engineer community (or link to communities they might find useful).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I quite like that the concluding chapter now sums up the skills gained by each of the personas. I think that is both expected and useful. I agree that including next step for the reader (not necessarily for each persona) is something that would be helpful and it could nicely round of the chapter. Either mentioning specific skill and linking to resources that covers some of these or suggesting strategies for how the reader can work to improve their RSE skills on their own after mastering this book's content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #563.
exercise. I am at the beginning and I think active titles are easier for me to | ||
consume (or make the choice that I want to spend time reading). | ||
|
||
d. I think Chapter 8 should directly reference Make. I might say there are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The title for the automation chapter does now explicitly reference Make.
c. Noticing a trend of noun-heavy titles. Could Sec 1.4 have a verb---"Setting | ||
up your project"? The current title might make more sense to me deep inside an | ||
exercise. I am at the beginning and I think active titles are easier for me to | ||
consume (or make the choice that I want to spend time reading). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of our Chapters have active titles and most of the Sections do, but not all of the Sections. Should we aim for active titles for all Sections (besides template sections like Key Points and Exercises)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👎 - I think what we have is good enough to ship.
It looks like most people have read the review. In terms of potential changes arising from our comments, it looks like we have unanimous agreement on:
There is conflicting or lukewarm opinions on:
If anyone wants to argue strongly in support of one of the conflicting/lukewarm issues feel free, otherwise I guess things stay as they are and we can merge this review :-) |
👍 to all of this. |
No description provided.