Skip to content
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

Feedback from the training: Summary and Setup episode #105

Closed
24 of 25 tasks
ehogan opened this issue Jan 10, 2025 · 1 comment · Fixed by #106
Closed
24 of 25 tasks

Feedback from the training: Summary and Setup episode #105

ehogan opened this issue Jan 10, 2025 · 1 comment · Fixed by #106

Comments

@ehogan
Copy link
Member

ehogan commented Jan 10, 2025

As part of #46, I will work through the lesson with the intention of ensuring it is suitable for a independent learner i.e. without the presence of an instructor. I will also ensure that the topics mentioned at https://github.com/MetOffice/Science-Git-Migration-Project/issues/56#issuecomment-2511167781 are covered. As requested by @astroDimitrios, I will open an issue for episode.

First paragraph

DT: I have added in learning outcomes as a section under the pre-requisites.

Prerequisites

  • Unix Shell --> command line? (Windows doesn't use Unix)
  • Should the prerequisites include things like "No knowledge of Git and GitHub is required." and "Access to the command line and a browser connected to the Internet are required."? We discussed adding "This lesson has been fully tested on Linux. Learners using either Windows or MacOS should let their instructor know.". Perhaps adding "or e-mail the support mailbox" for independent learners, or some other clarifying information?

Pre-workshop Survery

  • Should pre-workshop be pre-training or pre-lesson?
  • Include a link to the pre-workshop survey (independent learners will not receive a calendar invite containing this information).

Installing Git

  • I'm debating whether to remove the "Installing Git" section? Or perhaps start with the git --version command, then have a box with information about installation (maybe some external learners will need it?).
  • Remove the Git installation on links; they appear to be unnecessary as they are all the same and all point to the same link as the one in the previous sentence.

DT: I have re-worked this section as tabs for MO Linux, MO Windows, and Non-MO Systems

Creating a GitHub Account

  • episodes 7 & 8 no longer refer to the GitHub episodes. Is it possible to include Git and GitHub subsections? Then the subsection could be refered to instead.

DT: The infrastructure doesn't have subsections but I have changed the wording.

Multi-factor Authentication

  • multi-factor authentication is MFA rather than 2FA (which is two-factor authentication).
  • There is information about how to use KeePass at the MO. It's a MO-specific link though. Would it be worth including it?

Optional: Git Autocomplete

  • I wonder whether phrases like Your instructor may point you to another online resource for your OS. should be removed, as independent learners won't have this? Maybe we need a "contact one of the Git and GitHub champions" statement instead?
  • Please do Should the autocomplete instructions mention creating the ~/.bashrc.d directory? #104, with a mkdir example 😊
  • Even though the Azure SPICE VDI documentation recommends naming the file git.bash, since it is setting the prompt, I would propose using prompt.bash instead.
  • Use GIT_PROMPT_PATH=/usr/share/git-core/contrib/completion/git-prompt.sh, as specified in the Azure SPICE VDI documentation, as this path works on both the current VDI and Azure SPICE VDI.
  • There is extra whitespace at the end of if [[ -r "${GIT_PROMPT_PATH}" ]]; then (which is noticable when copying the text).
  • There are other differences between this version of the script and the version in the the Azure SPICE VDI documentation. Would it be possible to make these consistent / link from one to the other (single source of truth?).
  • Would it be worth adding that noticeable changes won't be seen to the prompt until located in a directory containing a Git repository?

DT: All changes our side are done.

GIT_PROMPT_PATH

  • The following paths are for Met Office colleagues. If you are external to the Met Office ... --> The paths above are for Met Office systems. If you are not using Met Office systems .... Then remove the paths as we determined (above) a path that is the same on both current VDI and Azure SPICE VDI.

If you have already modified your PS1

  • There is mention of PROMPT_COMMAND, but this doesn't exist in the code above.
  • Use prompt rather than PS1, unless it a command line command.

DT: This mentiond prompt_command because if you have set PS1 already then you can set prompt_command instead.

How to get help

  • Would it be possible for the first thing to be "attend a Git and GitHub Surgery" and the second thing be e-mail the support mailbox, please? Neither of these are MO-specific, so the text about "at the MO" can be removed. Also, remove the instructor comment (as mentioned previously)?
  • Science Git Migration Project SharePoint homepage and MetNets homepage. --> Science Git Migration Project Comms Site. (it links to MetNet, and everyone should have / be able to get access to the Comms Site).

Add in PROMPT_DIRTRIM

  • Instead of Add in PROMPT_DIRTRIM as a subsection, would something like Trim long directory paths be more descriptive?

Add in a newline

  • Is the (conda-env) part needed? It might cause confusion, since it wasn't in the previous examples.
@astroDimitrios
Copy link
Member

Made a few comments inline above and have opened #106

astroDimitrios added a commit that referenced this issue Jan 20, 2025
* Addresses review comments on Issue 105

* Address review comments

* Add version number disclaimer and note about nano and cat command usage
github-actions bot pushed a commit to astroDimitrios/git-novice that referenced this issue Jan 20, 2025
Auto-generated via `{sandpaper}`
Source  : 909f4e9
Branch  : main
Author  : Dimitrios Theodorakis <[email protected]>
Time    : 2025-01-20 13:30:27 +0000
Message : MetOffice#106 Addresses review comments on Issue MetOffice#105 Summary and Setup feedback

* Addresses review comments on Issue 105

* Address review comments

* Add version number disclaimer and note about nano and cat command usage
github-actions bot pushed a commit to astroDimitrios/git-novice that referenced this issue Jan 20, 2025
Auto-generated via `{sandpaper}`
Source  : 858da6b
Branch  : md-outputs
Author  : GitHub Actions <[email protected]>
Time    : 2025-01-20 13:32:13 +0000
Message : markdown source builds

Auto-generated via `{sandpaper}`
Source  : 909f4e9
Branch  : main
Author  : Dimitrios Theodorakis <[email protected]>
Time    : 2025-01-20 13:30:27 +0000
Message : MetOffice#106 Addresses review comments on Issue MetOffice#105 Summary and Setup feedback

* Addresses review comments on Issue 105

* Address review comments

* Add version number disclaimer and note about nano and cat command usage
github-actions bot pushed a commit to astroDimitrios/git-novice that referenced this issue Jan 21, 2025
Auto-generated via `{sandpaper}`
Source  : 858da6b
Branch  : md-outputs
Author  : GitHub Actions <[email protected]>
Time    : 2025-01-20 13:32:13 +0000
Message : markdown source builds

Auto-generated via `{sandpaper}`
Source  : 909f4e9
Branch  : main
Author  : Dimitrios Theodorakis <[email protected]>
Time    : 2025-01-20 13:30:27 +0000
Message : MetOffice#106 Addresses review comments on Issue MetOffice#105 Summary and Setup feedback

* Addresses review comments on Issue 105

* Address review comments

* Add version number disclaimer and note about nano and cat command usage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants