Review iteration Course Steps

[ppremk]

Hey @hectorsector here are the proposed flow based on the changes which I have tested on this repo. The commit history would help tell the story. @a-a-ron this might help giving you a little context on the flow of the course and help in writing the response.

Some steps were either combined or dropped in this additional feedback based on the first round of logic iteration that you have done in the closed PR Address feedback for first round of logic #33

Course Steps (new template branch master)

1. Enable continuous integration (new template branch)

0. Enable GitHub Pages (pointing to master/docs without theme selection)
1. Login to CircleCI with a valid account, add a new project CI, follow instructions below
2. Step Create a folder named .circleci and add a file config.yml (so that the file path be in .circleci/config.yml). (this step is executed in GitHub)
3. Populate the config.yml with the contents of the sample .yml (this step is executed in GitHub)
4. Push this change up to GitHub. (this step is executed in GitHub) 4.1. Is it better to do these changes via a PR (new user created branch) using the GitHubFlow 4.2. Merge the PR to master branch
5. Start building! This will launch project on CircleCI and make webhooks listen for updates to the project. (This step is executed in CircleCI)

Note: Steps 1 to 5 was required to be able to start building directly in Circle CI and also trigger the status.context==ci/circlecir. Verified to build a new CI using a new repo without reproducing steps 1 to 5 the the CI project did not build

2. Configure your CI provider

1. this step would be not required since steps 1.1 to 1.5 covers this requirement

3. Merge the configuration

1. this step would be not required since steps 1.1 to 1.5 covers this requirement

4. Add Validation Enable/Configure Test Automation (new template branch)

1. In this step proposal to introduce setting up Test Automation
2. Step to add Gemfile with taks to add htmlprrofer and supporting dependencies
3. Step to add Rakefile with basic task which runs a shell command to echo out some text
4. Step to edit .circleci/config.yml line which replaces the echo out command with bundle execute rake test

Note: The Gemfile.lock is a generated file when the project is compiled, I am in two minds about adding this file as part of this step. Will need to test if the test works without the Gemfile.lock

5. Protect a branch

1. Step to configure branch protection rule
2. Step to enable "Require status checks to pass before merging" --> ci/circleci: build
3. Step to introduce a simple PR merging a passing build (maybe adding a new MD file)
4. Merge the PR with successful build

6. Fix the broken build Add/Configure Test (new template branch)

1. Create new branch add test
2. Step to add the script/test.rb file (copy paste)
3. Step to edit the Rakefile (copy paste code to trigger the test from step 6.1)
4. Merge the changes via a new PR

7. Merge code with a successful build Fix broken build (new template branch)

1. Step to create new branch with a broken link, highlighting the failing CI build status

8. Add a unit test Merge code with a successful build

1. Step to merge code with fixed link by user

9. Merge your unit test Deploy

1. Step to highlight the deployment to GH pages

10. Deploy

[hectorsector]

Thanks for the detailed write up @ppremk. My opinions may change as I implement the suggestions, but here are my thoughts so far:

1. Enable continuous integration (new template branch)

✨ I noticed that you even completed the stage so we don't get a build error THANK YOU!

• Enable GitHub Pages (pointing to master/docs without theme selection)

I'm not sure this makes sense right off the bat. It introduces a CD concept first in a CI course. I like doing it at the end as the cherry on the cake.

3. Step Create a folder named .circleci and add a file config.yml (so that the file path be in .circleci/config.yml). (this step is executed in GitHub)

Then we don't need to provide the file on a branch, do we?

4. Add Validation Enable/Configure Test Automation (new template branch)

Looks ✨

• Step to add Gemfile with taks to add htmlprrofer and supporting dependencies
• Step to add Rakefile with basic task which runs a shell command to echo out some text

I'm not sure I want to devote so many steps to language specific items (like a Gemfile and Rakefile). But, then again, it's something we can improve upon as we add support for more languages. How awesome would it be if we had a template repo for Ruby projects, and another for Node, and another for Go, and so on.

Note: The Gemfile.lock is a generated file when the project is compiled, I am in two minds about adding this file as part of this step. Will need to test if the test works without the Gemfile.lock

Gemfile.lock should be committed to the repository, and it should be generated by bundle install. Without making learners run bundle install locally, I think the best course of action is to introduce it for them on a branch.

• Step to enable "Require status checks to pass before merging" --> ci/circleci: build

Thanks for checking on this. I've been trying Circle CI with the Checks API and, in that case, we should check for a ci/circleci: workflow.

@ppremk I think somewhere along the way we lost a test. I envisioned a link checker to be an initial use of CI, but I also envisioned a second use of CI where we would add a new feature to docsify (for example, adding a cover page) and then a unit test to ensure that feature works (for example, visiting the deployed URL should return the cover page). Does that make sense?

[hectorsector]

@ppremk for Step 1: Enable continuous integration, it won't work without a Gemfile:

This is the output from Circle:

bundle install --jobs=4 --retry=3 --path vendor/bundle
Could not locate Gemfile
Exited with code 10

I think this is because the Gemfile is used for bundle install, which you call in the CircleCI config.

[hectorsector]

@ppremk let's change this as follows:

4. Add Validation Enable/Configure Test Automation (new template branch)

^ let's keep using the branch: configure-test-automation

2. Step to add Gemfile with taks to add htmlprrofer and supporting dependencies
3. Step to add Rakefile with basic task which runs a shell command to echo out some text

^ let's do this for the learner on the branch automatically

4. Step to edit .circleci/config.yml line which replaces the echo out command with bundle execute rake test

^ this is the part we'll need the learner to do manually, please remove it from the branch

[hectorsector]

:wave: @ppremk I’ve spent some time working on this and think I can make the changes I described. However, I’m still spending some time thinking about the unit test.

[ppremk]

I'm not sure this makes sense right off the bat. It introduces a CD concept first in a CI course. I like doing it at the end as the cherry on the cake.

💯 agreed

Then we don't need to provide the file on a branch, do we?

We could provide a blank config.yml file and as an activity get the users to fill it out via copy paste?

I'm not sure I want to devote so many steps to language specific items (like a Gemfile and Rakefile). But, then again, it's something we can improve upon as we add support for more languages. How awesome would it be if we had a template repo for Ruby projects, and another for Node, and another for Go, and so on.

These steps are proposals, we can introduce these files and skip manual steps where necessary, so I would go with your thoughts and flow 🙂

Gemfile.lock should be committed to the repository, and it should be generated by bundle install. Without making learners run bundle install locally, I think the best course of action is to introduce it for them on a branch.

💯 agreed as well since we are not having any local instance of the project. I had also checked the circleci ci and noticed that the steps still get executed since there is a fail safe step to compile and build the dependency if no Gemfile.lock is found. This also makes the build a little slow

@ppremk I think somewhere along the way we lost a test. I envisioned a link checker to be an initial use of CI, but I also envisioned a second use of CI where we would add a new feature to docsify (for example, adding a cover page) and then a unit test to ensure that feature works (for example, visiting the deployed URL should return the cover page). Does that make sense?

Let's sync on this to see how we can extend existing steps with an extra test case 🙂

[ppremk]

^ let's keep using the branch: configure-test-automation

👍

^ let's do this for the learner on the branch automatically

These files are already in there. I will proceed to add the Gemfile.lock

^ this is the part we'll need the learner to do manually, please remove it from the branch

What say you if we get the end user to update the syntax with the correct snippet? We need this file in the previous step for it to pass the build

[ppremk]

👋 @ppremk I’ve spent some time working on this and think I can make the changes I described. However, I’m still spending some time thinking about the unit test.

Let's sync on this. 🙂

[ppremk]

@ppremk for Step 1: Enable continuous integration, it won't work without a Gemfile:

Would you be ok if this line was commented out first and then in the subsequent steps it is enabled?

[hectorsector]

@ppremk I spent some time today building on your amazing work and created this template repo. I think any of the quirks that I brought up here are addressed there. The course is also on staging using the new template. Feel free to close this issue unless there's remaining items you want to address. I'm also going to close any remaining template repo issues. I'll open new ones later for tasks like ensuring that all commits are made by githubteacher.

[ppremk]

@hectorsector thats really awesome! I will peek into the template repo and also review the course flow on staging. Based on the template name, I am assuming that we will be having an additional template for the travis ci course? I shall keep an eye out for any new issues you will post 🙂

[ppremk]

closing in favor of #38