Skip to content

Conversation

@JasonEtco
Copy link
Contributor

@JasonEtco JasonEtco commented Aug 29, 2017

This PR adds a number of new graphics to the training-manual courses. Will close #47.

  • Forks
  • git bisect
  • Multiple remotes
  • Difference between origin/upstream
  • Difference between forks/branch/clone
  • Resolving Merge Conflicts
  • Merging Pull Requests
  • Additional git reset

cc @githubtraining/trainers

Copy link
Contributor

@hollenberry hollenberry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JasonEtco These are great!

  • Passing thought for book/img/clone-branch-fork.png: Would it be possible to put the repository image inside the image of the monitor?
  • I love the use of the images for fork and branch from the UI.
  • In looking at Git Bisect, I'm wondering if it would be possible to switch the view to be horizontal. In the GitHub visualization tool, history is displayed horizontally and it might help comprehension to keep that consistent.
  • Nice work!

@JasonEtco
Copy link
Contributor Author

Passing thought for book/img/clone-branch-fork.png: Would it be possible to put the repository image inside the image of the monitor?

I'd rather keep the icons separate (rather than composing new ones) but I see what you're after here; I'll do something along those lines 👍

@JasonEtco
Copy link
Contributor Author

Note that I'm adding these images, but I'm not including them or linking to them anywhere. @brianamarie @hollenberry y'all will need to figure this out 👍

@brianamarie
Copy link
Contributor

@githubtraining/trainers I have incorporated @JasonEtco's ✨ graphics into the manual, both inline and with an appendix on forking.

Your review would be greatly appreciated, specifically on:

  • Image placement
  • Content and wording for forking appendix

Copy link
Contributor

@hollenberry hollenberry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brianamarie Built locally and this looks good. One thought: what do you think about moving the fork and pull section to just before Workflow Review, in section 3?

@brianamarie
Copy link
Contributor

One thought: what do you think about moving the fork and pull section to just before Workflow Review, in section 3?

@hollenberry My understanding is that we don't always cover Fork and Pull. If the customer isn't using Fork and Pull, we will just use the script to generate their own repositories and we won't really get into Fork and Pull at all. Is this correct?

@hollenberry
Copy link
Contributor

👋 @brianamarie . That's my understanding as well.

Is the scripted step our default/expected path, as opposed to Fork + Pull? My understanding is that we often pick one or the other based on customer, but I'm not sure which is most consistent.

For those using the manual outside of a class with script permissions, a link to the appendix might be valuable.

@brianamarie
Copy link
Contributor

For those using the manual outside of a class with script permissions, a link to the appendix might be valuable.

👍 This sounds like a good solution. I'm pushing up a commit doing just that. What do you think?

@hollenberry
Copy link
Contributor

Works for me.

I'm still lacking clarity on default class approaches, but this can ship without that. 👍

@brianamarie
Copy link
Contributor

I'm still lacking clarity on default class approaches

I agree 👍 I think more clarity here would be good.

@brianamarie brianamarie merged commit d5af19c into master Sep 20, 2017
@brianamarie brianamarie deleted the new-graphics branch September 20, 2017 09:15
@crichID
Copy link
Contributor

crichID commented Sep 20, 2017

I'm still lacking clarity on default class approaches

@hollenberry @brianamarie should we open an issue to discuss this and perhaps put a team meeting label on it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New Training Manual Graphics

5 participants