Hello @hectorsector! I looked through this course with the most recent changes from the full-responses branch. Here's my feedback - you'll notice it's a lot more sparse at the end, partially because it was a lot of repetitive feedback, and partially because I think a lot of it is more detailed about the responses than I think you're asking for at this point in time.

Let me know if you have any questions about this or if you'd like to walk through it synchronously. Hopefully this is helpful!

General

I think the names of the steps can be more specific and contextualized in terms of the broader workflow and concepts we're trying to teach
Since this course is advertised as CI, we should be incorporating the CI concepts into each step, referencing how this specific technical example serves a larger practice

(0) Step 1

What is a workflow and an action? Even if these are explained in more detail later in the course, as a learner, if I saw this amount of information with these steps, I would assume that I don't know enough to proceed. We should include some level of explanation at this point: even if it's rudimentary and it says we'll go into more detail later. Currently there's no conceptual learning happening with this step at all.

Step 2

If I name the PR something inexact, I get an error. Is that core to what this course is trying to teach? Is there a reason we care so much? It doesn't seem related to the topic at hand.
In 01_explain-template.md
- We should specify which file
- We also should specify what is being run, not just "CI"
- We should advise the users on how long they should expect to wait
- We should let the learner know that they can expect more clarification about the points in the file below in the line comments.
Step 2 isn't actually anything that the learner does. As a learner, I expect that I should complete an action to move forward every step. Can we combine steps 2 and 3? I think a wait step in part 2 that waits for the feedback for the action, then moves on, and gives instructions, would solve it. Ex: circleCi course
Why is the pull request for jest being created at this point? As a learner, I think I'm waiting for the Actions to be completed - so it's unclear why the Jest PR is being created now. It doesn't look like we're referencing to it or asking the learner to look at it yet, either. Can this be created later?
- Do we have the learner create a pull request, then merge a different pull request? Why? Does this mimic a typical workflow? If we want them to appear, why not have the bot merge? If we want the learner go to through the pull request, why not merge the first PR first? I understand the flow we're creating but it seems like an opportunity for confusion that we can avoid - I think it makes more sense to have it all committed and merged automatically into the same PR.

Step 3

I don't think this merge should be something the learner does - I think it confuses the natural workflow that we recommend. I'm a fan of the method of calling out specific things through line comments and explaining them. I recommend we merge this automatically once the build succeeds, and point out separately at this point the successful build, what it has and doesn't have, and the new commits that we made through merge. I think the flow of having them merge would definitely point out the changes, but I think it would confuse the learner...it confused me. OR, like above, I think these should be two entirely different pull requests that we have the learner merge chronologically, or both to master.

Step 4

I LOVE the authentication step of having the learner type the name of one of the failed tests. :sparkles:

Step 5

I think it's great that we have the user introduce the tests, but I think that introducing a workflow and the action + fixing the build don't necessarily make sense in the same PR. I understand that we always want master to have a passing build, but I would always recommend to leaners to keep their PRs logically grouped. I think introducing an action and fixing a bug belong in separate PRs.

Step 6

Same notes as before about content breadth of each pull request

Step 7

It's very unclear to me what editing the version of node has to do with implementing a new CI workflow. How are these related? What are the learning outcomes of this step?
Same feedback as before - does it really matter what the pull request title is? Is that directly related to the learning goal, strongly enough that I'm blocked if I don't do it? If we're enforcing this practice, are we equally enforcing all of the other practices that users should be doing, like self-assigning or including content in the body of the PR?

Step 8

This step starts to become more clear as to what we're trying to accomplish with this workflow. I think the specific requirements for what this workflow is trying to accomplish should be called out more loudly, and the concepts that we're trying to teach about CI should also be called out here. Ex; why does it matter with CI practices to include this level of detail in our actions and test? Why does this help?

Step 9

This step seems to add a lot. Similarly to how the steps were called out separately in step 1, I think these would be much more approachable if they're broken up into their own steps. Here would be the place I'd expect more detail and information on the possibilities of these fields, the syntax requirements, different examples, etc.

Step 10

Same feedback as step 2 - can we let this be a waiting gate for the previous step vs a step that implies the learner has to do something?

Step 11

:+1: This is the level of granularity per step that would benefit step 9.
Are we adding an action to the workflow file? This is a great opportunity to reinforce that vocabulary.

Step 12

Same for step 11

Step 13

Same general feedback about opportunity to reinforce the specific technical wins here and how they map to broader CI concepts

Step 14

Love the checklist - could this checklist be in an issue to mimic the practices we'd recommend for a team? I think that would be a nice touch and make it more realistic
Why do some things belong in different actions and some things belong in different workflows? What necessitates a different workflow? This should be explained here

Step 15

Love the modularity of this step

Step 16

Love the modularity of this step, and that it's reinforcing the same concepts in different ways with different examples

Step 17

This step seems to add a lot more than the previous steps - we should be explaining line by line after or before they commit

Step 18

We're asking the user to set branch protections, but not really testing that. I like the workaround that you did here to make it work.

githubtraining / using-github-actions-for-ci

Course flow feedback #13