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 risk assesments #600

Merged
merged 4 commits into from
Feb 5, 2025
Merged

Update risk assesments #600

merged 4 commits into from
Feb 5, 2025

Conversation

ShadowLord2005
Copy link
Contributor

I believe this closes out the last part of srobo/tasks#1476

Copy link
Member

@PeterJCLaw PeterJCLaw left a comment

Choose a reason for hiding this comment

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

Thanks! A couple of small suggestions inline.

Copy link
Member

Choose a reason for hiding this comment

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

nit: the mix of underscores and dashes here feels weird. It looks like we've used underscores in risk assessment file names in the past, perhaps we could stick with that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines 15 to 17
If you have any questions please contact us at [{{ site.emails.teams }}][teams-email].

[teams-email]: mailto:{{ site.emails.teams }}
Copy link
Member

Choose a reason for hiding this comment

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

Please can we keep this footer? (about contacting us if any questions)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you have any questions please contact us at [{{ site.emails.teams }}][teams-email].

[teams-email]: mailto:{{ site.emails.teams }}
This year our Tech Days are taking place in two different locations. We have published a risk assessment for both [Cambridge]({{ '/resources/sr2025/risk-assessments/cambridge_tech-day.pdf' | prepend: site.baseurl }}) and [Horsham]({{ '/resources/sr2025/risk-assessments/horsham_tech-day.pdf' | prepend: site.baseurl }})
Copy link
Member

Choose a reason for hiding this comment

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

Missing full stop at the end of this sentence?

Suggested change
This year our Tech Days are taking place in two different locations. We have published a risk assessment for both [Cambridge]({{ '/resources/sr2025/risk-assessments/cambridge_tech-day.pdf' | prepend: site.baseurl }}) and [Horsham]({{ '/resources/sr2025/risk-assessments/horsham_tech-day.pdf' | prepend: site.baseurl }})
This year our Tech Days are taking place in two different locations. We have published a risk assessment for both [Cambridge]({{ '/resources/sr2025/risk-assessments/cambridge_tech-day.pdf' | prepend: site.baseurl }}) and [Horsham]({{ '/resources/sr2025/risk-assessments/horsham_tech-day.pdf' | prepend: site.baseurl }}).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Meta for future: when importing files from other sources it's generally a good idea to include in the commit message what the source was. This both helps review (means that reviewers can just check that the file matches what the source was rather than having to review the whole content) and means that future updates are easier (since getting back to the source is easier).

For now just adding the links to the PR is probably enough, though if you wanted to edit the commit to add the links that's also an option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll remember that in future. Not sure how to edit commits.

@ShadowLord2005 ShadowLord2005 merged commit 69f230f into main Feb 5, 2025
5 checks passed
@ShadowLord2005 ShadowLord2005 deleted the update-risk-assesments branch February 5, 2025 21:34
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.

3 participants