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

Update lecturer_s_procedure_ #375

Merged
merged 12 commits into from
Jan 10, 2024
Merged

Update lecturer_s_procedure_ #375

merged 12 commits into from
Jan 10, 2024

Conversation

mselwa
Copy link
Contributor

@mselwa mselwa commented Dec 7, 2023

No description provided.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…sselt.rst
@mselwa mselwa requested review from moravveji and jooghe December 7, 2023 10:38

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…sselt.rst

added MFA login info
Copy link
Contributor

@jooghe jooghe left a comment

Choose a reason for hiding this comment

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

added some comments, can you have a look?

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…sselt.rst

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…sselt.rst

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…sselt.rst
@moravveji
Copy link
Contributor

@mselwa : thanks for ironing out this page.
I have some remarks, but I can hardly edit in between the lines, as the review pane only shows the modified content, and not the full text.
Is it OK if I fetch this branch, make some slight changes, and then push it to this PR?

@jooghe
Copy link
Contributor

jooghe commented Dec 14, 2023

@mselwa : thanks for ironing out this page. I have some remarks, but I can hardly edit in between the lines, as the review pane only shows the modified content, and not the full text. Is it OK if I fetch this branch, make some slight changes, and then push it to this PR?

Ehsan, that should be no problem.Can always be reverted.

@moravveji
Copy link
Contributor

With the commit 3bb20cd, I have made some slight changes to the wordings, and also have cut long sentences into shorter ones.

Copy link
Contributor

@moravveji moravveji left a comment

Choose a reason for hiding this comment

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

This is now good to go for me.

@jooghe
Copy link
Contributor

jooghe commented Dec 19, 2023

I still have some comments. It is not clear that we provision the account. That should be explicit in a seperate bulletpoint 2, not a bit hidden as it is now after the reference to ondemand.

@jooghe
Copy link
Contributor

jooghe commented Jan 9, 2024

I added an additional point about automatic account creation. Can you have a look? We still need to be more clear on the use of Filezilla and NX, if it is enough to have an active ssh-agent with a certificate. But I think we best resolve that on the general access page.

@moravveji
Copy link
Contributor

I added an additional point about automatic account creation. Can you have a look? We still need to be more clear on the use of Filezilla and NX, if it is enough to have an active ssh-agent with a certificate. But I think we best resolve that on the general access page.

@jooghe : your changes look good to me.
Regarding the use of NX and FileZilla, I added an extra sentence, pointing to the configuration pages for NoMachine and FileZilla on the VSC docs.

facilities are necessary for teaching the course. It is mandatory to provide a list
of students with their full names, KU Leuven/UHasselt email addresses, and their student number.
For KU Leuven students, you may convert the students' q-number to their r-/s-number using the export
tool on Toledo (Administrations -> Convert user-ids (q-numbers))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just jumping in to mention that once https://gitea.icts.kuleuven.be/foz/foz-vsc-accountpage-automation/pulls/7 is approved, the conversion of q-numbers is no longer necessary: the updated account creation script can deal with q-numbers.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, this page will require another quick fix after that PR is merged and comes into production.

@mselwa mselwa merged commit 2b61b1c into master Jan 10, 2024
2 checks passed
@mselwa mselwa deleted the mselwa-patch-lecturers branch January 10, 2024 12:43
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 this pull request may close these issues.

None yet

4 participants