First draft responses - Githubissues

githubtraining / ci-circle

Course repo for Learning Lab course "Continuous Integration with CircleCI". Template repo ➡

https://github.com/githubtraining/ci-circle-template

Creative Commons Attribution 4.0 International

3 stars 9 forks source link

First draft responses #34

Closed a-a-ron closed 5 years ago

a-a-ron commented 5 years ago

In response to issue #6, this PR adds content to the following responses files;

Will update list when files are completed

[x] 01.1_welcome.md
[x] 01.2_congratulations.md
[x] 01.3_next-steps.md
[x] 02.1_build-status.md
[x] 02.3_go-to-customize-build.md
[x] 03.1_customize-the-build.md
[x] 03.2_add-validation.md
[x] 03.3_protect-branch.md
[x] 03.4_branch-protected.md
[x] 03.5_fixed.md
[x] 03.5_go-to-feature-pr.md
[x] 05.1_feature.md
[x] 05.2_merge.md
[x] 05.3_goto-deployment.md
[x] 06.1_deploy.md
[x] 06.2_see-deployed.md
[x] 07.1_congratulations.md

a-a-ron commented 5 years ago

@hectorsector @ppremk, based on your thoughts and feedback in issue https://github.com/githubtraining/ci/issues/35, it sounds like the course workflow is still being developed. I also got stuck last night on response file 05.1_feature.md as the feature shows as TBD so i'm not sure what we want here. I'm thinking it may be best to wait for a more solidified course flow before continuing?

hectorsector commented 5 years ago

@a-a-ron I merged in master, and resolved most of the conflicts that occurred from it. I was able to combine your responses with most of the new logic, but for those I couldn't I left my notes. Hopefully this makes it easier for you.

You can also use the most recent logic on staging.

a-a-ron commented 5 years ago

Hey @hollenberry, if you have some time to review, i'd love your 👀 before it's merged. What i'm looking for is that the response files match up with the config and that there isn't any glaring grammar or spelling mistakes.

Once those are validated, merging will probably make it easier to get a feel for the response flow rather than reviewing it here.

brianamarie commented 5 years ago

@a-a-ron I'm going through now and making changes based on @hectorsector's comments. If there are any that I can't address, I'll make note of it. I'm hoping that once I do that, I can go through with fresh eyes and review comparing the responses with the config. 👍

Note: this is the one that I'm not sure about. I've made changes for the others and will push that commit soon.

This is the OP for the PR. It is directly followed by a separate comment in the PR (with a suggested change). I recommend devoting the OP to why we'd want to include the build process as part of our CI config, and explaining how building works in Jekyll, and leave the second comment as the mechanics of how to do so.

brianamarie commented 5 years ago

@a-a-ron At this point, given the timeline and where we currently are, I recommend merging this as is so that others can test in staging and get a better feel for everything.

The point that I couldn't figure out from @hectorsector's notes, and other feedback, could be addressed in new issues and PRs. What do you think?

ppremk commented 5 years ago

proceeding to merge based on the go ahead from @a-a-ron and 👍 from @hectorsector and @brianamarie