New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update keyboard key references with Carpentries style #342

Open
jcoliver opened this Issue Jan 26, 2018 · 7 comments

Comments

Projects
None yet
4 participants
@jcoliver
Collaborator

jcoliver commented Jan 26, 2018

Some episodes (especial 1 & 2) could use updating to change references to keyboard keys, such as SHIFT and CTRL to conform to Carpentry Style (<kbd>Shift</kbd> and <kbd>Ctrl</kbd>, respectively). See the style guide for more details.

@jcoliver

This comment has been minimized.

Collaborator

jcoliver commented Jan 26, 2018

@naupaka This is a newcomer-friendly issue, but I think I lack permissions to apply labels.

@norcalbiostat

This comment has been minimized.

norcalbiostat commented Jan 26, 2018

I don't think I did it right. I can't compare to the master branch (as requested by the contributing instructions), and I only changed this one file - yet it's saying I want to merge 5 commits (4 of which were from 2 years ago from another person).
This contribution is surely newcomer friendly, but the instructions on how to correctly do a PR is not newbie-to-git friendly :(

@norcalbiostat

This comment has been minimized.

norcalbiostat commented Jan 27, 2018

and this fix was merged an hour ago by someone else (since mine didn't work I guess). Can someone close this now? This is the second issue that i've tried to fix that has already been addressed. Is there some guidance for contributors to at least state that they're working on an issue so others don't waste time trying to fix something that someone else has already done?

@naupaka

This comment has been minimized.

Member

naupaka commented Jan 27, 2018

@norcalbiostat Thank you for your PR! I'm very sorry it got confusing (and I definitely agree git is pretty far from intuitive!). One of the things we are working on this quarter is a more Carpentry-oriented guide for making contributions, in addition to the general guides out there on the Internet.

I think the issue causing the merge conflicts is that you had forked the lesson a while back, and the version you made changes to was an old one instead of the newest, which is why you saw all the merge conflicts. The other possibility is that you made changes to master or some other branch instead of gh-pages.

To get the newest changes from this repo, there are two options (and forgive me if you know all of this already, but just in case I'll talk it out):

  1. before you make any updates of your own, in your local copy of your forked version of this repository, on your computer:
# This adds the address to the main SWC repo of the lessons
git remote add upstream https://github.com/swcarpentry/r-novice-gapminder.git

# this checks out the gh-pages branch locally on your machine so changes we pull down
# get added to it (instead of, e.g., the master branch)
git checkout gh-pages

# this pull down all the new changes from the SWC repo and merges them on top of your
# local changes
git pull upstream gh-pages

# this pushes all the updated content back to your fork of the repo, which is 
# under your account on GitHub
git push origin gh-pages

Now you have all of the newest changes both locally and on your fork.

Here's a visual:

github_contribution_workflow

Or, 2) delete your fork on GitHub and then re-fork it, so you have a copy of all the newest changes. Then make your change as usual and open a PR.

Then any time you want to make a new contribution to the repo, just repeat steps 3-6 in the diagram.

Also, I think the documentation is out-of-date, since we just use the gh-pages branch directly now, and don't ever use the master branch. So thank you for pointing that out, we'll need to get that fixed ASAP.

I was also in the middle of fixing the merge conflicts on your PR locally so that your contribution could be merged and your contribution recognized, and was also surprised to see the other PR with a fix to the same issue submitted and merged before yours.

That's a coordination issue that we (cc: @jcoliver @mawds) as maintainers of this repository need to get figured out (it's just been me for a while, and two new maintainers just started yesterday), and not the way things should work around here.

Once again, I'm sorry for the confusion and frustration, and I hope it doesn't discourage you from making future contributions! We're also always happy to help troubleshoot the contribution submission process anytime in an issue or a PR -- just tag one or all of us and we can take a look and help get things on track.

@jcoliver

This comment has been minimized.

Collaborator

jcoliver commented Jan 30, 2018

@naupaka Great visualization. Should be connected to #304 !

@nick-ulle

This comment has been minimized.

nick-ulle commented Jul 12, 2018

Has this issue been resolved already? Grepping around the repository, I couldn't find any episodes with keyboard shortcuts missing <kbd> tags.

@jcoliver

This comment has been minimized.

Collaborator

jcoliver commented Jul 12, 2018

Thanks, @nick-ulle. With one exception, it looks like it (There's an "Esc" in episode 1 that's missing the tags).

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