-
-
Notifications
You must be signed in to change notification settings - Fork 65
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
Updates and modifications from June 2024 run at UKAEA #371
Conversation
Okay, I have added most of the feedback that came out of our run. I will stop adding anything else to prevent this becoming too large of a changeset. The rest will go into new issues. This is ready for review. |
@anenadic are you happy to be the reviewer of this? |
_episodes/11-software-project.md
Outdated
> > Also make sure you select the **SSH** tab and not the **HTTPS** one. | ||
> > For this course, SSH is the preferred way of authenticating when sending your changes back to GitHub, | ||
> > but if you have already authenticated through HTTPS in the past, | ||
> > it is fine for you to continue using that method. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have removed the option to use HTTPS + auth tokens in the past and are asking students to use SSH for authentication. If we want to start supporting HTTPS again - we probably need instructions on how to cache the token in a local credential manager - which then complicates the setup (which also needs to be updated) and complicates running of the workshop (different authN methods, different issues, different credential managers).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following our in-person chat, I agree that for simplicity we should just stick with SSH since HTTPS is actually more complicated to manage now. I'll remove reference in an upcoming change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 1efc1a0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is looking good to me - the one concern I have is adding support for tools (WSL) or GitHub authN methods (HTTPs) which we are explicitly not supporting in setup. Perhaps we need to discuss if we need to branch out and support these other tools and also what are the benefits of allowing back HTTPS for GitHub authentication (when SSH is the preferred and simpler method and students will most likely use it in the future so they should have their machines set up for this).
|
||
![VS Code application with the list of extensions found by search term "python"](../fig/vs-code-python-extension.png){: .image-with-shadow width="800px" } | ||
|
||
### Using VS Code with Windows Subsystem for Linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are explicitly saying we are not supporting WSL in our setup instructions :-).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll tweak the language to make it explicit that this option is not supported for the course.
However, given the number of people who use WSL (even just at my org), I think it is justified to
mention this here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Language updated in 1efc1a0
…parts of the lesson
…or/bielsnohr/june-2024-ukaea-updates Updates and modifications from June 2024 run at UKAEA
Some minor corrections and amendments that came out of our most recent run of the course. This is just the first batch, and I need to go through our notes to make sure all changes have been captured. Happy to add those onto this PR or make a separate one.
Closes #369