ProjectPythia / pythia-foundations

Jupyterbook source for the Foundations collection
http://foundations.projectpythia.org
Apache License 2.0
61 stars 43 forks source link

Polishing GH branch chapter #241

Closed jukent closed 2 years ago

jukent commented 2 years ago

GH branches has overlap from GH pull requests. I'm shortening the merging branches section and pointing to the PR chapter.

github-actions[bot] commented 2 years ago

This pull request is being automatically built with GitHub Actions and Netlify. To see the status of your deployment, click below.

🔍 Git commit SHA: ab5523d41b3daae900c33836118957960aa2da19 ✅ Deployment Preview URL: https://625d7cfc7e5f4f556b8fcded--pythia-foundations.netlify.app

jukent commented 2 years ago

fixing broken link in #244

jukent commented 2 years ago

I ironically made a GitHub mistake in this PR. I went to pull this branch into my workflow branch (PR #242) and I thought only that PR would be affected, but this PR now has those edits as well (both branches were updated). I might just delete the workflow file from this branch.

jukent commented 2 years ago

@clyne I've addressed your comments. Let me know if you think it could still use work.

clyne commented 2 years ago

@clyne I've addressed your comments. Let me know if you think it could still use work.

@jukent , sorry, I'm still finding some of the figures confusing. Not terribly surprising though; I think the whole notion of multiple repos for a single project is probably the most confusing aspect of git:-). I'm struggling with trying to follow how each command changes the state of the repo(s). Given that the examples shown can all pretty much build on each, perhaps showing how the repos evolve with each command would be more illustrative. See what you think of the quick and dirty figures here.

The intent is that each git command that changes state in the text is reflected by a figure that shows the state of the repos before and after the command. Also, the figures completely separate the local and remote repos so there is less confusion about which repo the commands apply to.

jukent commented 2 years ago

@clyne I think those figures are great! Maybe we can minimize them to show ONLY the change step -- and then create a GIF that flows through all of the steps to have at the end

clyne commented 2 years ago

That sounds great!

jukent commented 2 years ago

@clyne I am working on getting an iMovie download so I can make the GIFs. Just a little update -- unfortunately my Apple ID was locked and they, for security reasons, make me wait to recover it. I requested the recovery on March 9th and won't be able to access my account until the 23rd.

clyne commented 2 years ago

I've learned that since COVID all we do is wait, but I would have thought that Apple had their act together :-). Thanks for the update.

clyne commented 2 years ago

@jukent FWIW I use ffmpeg to create animations from a sequence of images. It's open source, runs everywhere, and an industry leader.

jukent commented 2 years ago

Ooh I'll check that out! Thanks @clyne

kmpaul commented 2 years ago

@jukent: To get the link-checker action to pass, you may need to update your branch with the most recent changes in the main branch.

jukent commented 2 years ago

@clyne I'm a little confused by your diagram on slide 5. Is this implying that you merge your commits from your "newbranch" into "main" before pushing upstream. I don't think I work this way

kmpaul commented 2 years ago

Yeah, I agree with @jukent here. The fork-based workflow is that you push commits to a branch on your fork and then you merge your fork's branch into the upstream main. You should not be merging your fork's branch into your fork's main.

brian-rose commented 2 years ago

Yeah, I agree with @jukent here. The fork-based workflow is that you push commits to a branch on your fork and then you merge your fork's branch into the upstream main. You should not be merging your fork's branch into your fork's main.

Agreed +1

In this workflow, the only changes you should be making to your main branch is merging changes from upstream main.

clyne commented 2 years ago

The slides were quick-and-dirty and intended to be exemplars. Feel free to modify to better reflect reality as you see fit :-)

jukent commented 2 years ago

@clyne @kmpaul and I just spent some time iterating on clearer slides. This PR should now reflect the new images and gifs we made. Once it has built, I will re-request your review.

https://docs.google.com/presentation/d/1_0xuSQ9G27kkMxrcJX4eICy8j1j7vmYg0tYCrpFnpto/edit?usp=sharing

jukent commented 2 years ago

@clyne @brian-rose This is ready for review

kmpaul commented 2 years ago

@jukent: This is looking great! Thanks for all the work on this.

Some nit-picky details that I will leave to you to decide whether to act on:

I think the GIF animations might need to be modified so that people know where in the "slide deck" the image is. So, maybe a "slide number" in a corner somewhere so that people know where they are.

Also, I think that some of the GIF animations are just too slow. I think it would be better for them to be faster and just on an infinite loop.

Also, I wonder if we can lengthen the start and end delays of the animations. That is, if the "next slide" time is 2 seconds, then maybe don't change the first slide until after 3 seconds and don't loop after the last slide until 3 seconds. ...? I'm not sure how to do this. Although, you could accomplish it be having duplicate slides and a faster "next slide" time, like so. If you have slides A, B, and C, you might create a "slide deck" with:

A -> A -> A -> B -> B -> C -> C -> C

And with a 1 second transition time, that would result in slide A being displayed for 3 seconds, B for 2 seconds, and C for 3 seconds. ...Just a thought. But maybe that is too picky.

Lastly, I think the slides need to be cropped. The whitespace above and below the images is too large.

jukent commented 2 years ago

@kmpaul just added slide numbers, sped up the animation, and cropped the gifs.

jukent commented 2 years ago

Looks great, Julia. I've added a couple minor text edits to consider.

Thanks! These changes are incorporated now.

jukent commented 2 years ago

Is the source material for your new figures available? I'd like to reuse for the section on PRs. Thanks!

Thanks!

https://docs.google.com/presentation/d/1_0xuSQ9G27kkMxrcJX4eICy8j1j7vmYg0tYCrpFnpto/edit?usp=sharing