Conversation
mo-mguidolin
left a comment
There was a problem hiding this comment.
Some comments to address.
By the way, on some of my instructions I have something like:
- Great the user the first time with "Hello - I am an expert shell development"
So I know when the rules are loaded. However, you have to promp "hello" to see the text.
Maybe something like.
- After reading this instruction reply "Ready to help with SSD documentations" to the user.
```
And test if it works.
.github/copilot-instructions.md
Outdated
| and instructions for using GitHub Copilot and Copilot Chat effectively within | ||
| this repository. | ||
| You are an expert technical writer and developer for the `simulation-systems` | ||
| project. Follow these directives for all code generation and chat responses. |
There was a problem hiding this comment.
Follow these directive sentence is probably implicit since it is a copilot-instructions.md.
There was a problem hiding this comment.
Agreed. I'll add repository context to clarify the purpose.
.github/copilot-instructions.md
Outdated
| Welcome to the `simulation-systems` project! This guide provides best practices | ||
| and instructions for using GitHub Copilot and Copilot Chat effectively within | ||
| this repository. | ||
| You are an expert technical writer and developer for the `simulation-systems` |
There was a problem hiding this comment.
Maybe it is missing a litle of context of what the repository is about This repository contains the working practices and what is the purpose of the work that Copilot should on this repo.
There was a problem hiding this comment.
Good point - this can be simplified.
.github/copilot-instructions.md
Outdated
| - Committing changes directly to `main` without review | ||
| - **Primary Format:** Always use reStructuredText (`.rst`) for documentation. | ||
| Never suggest Markdown unless specifically requested. | ||
| - **Scope Constraint:** Focus changes on the `source/` directory. Do not modify |
There was a problem hiding this comment.
Focus changes sentence is not clear. Also why only focusing on source directory?
I would go something like
- **Scope Constraint:** Modify only the documentation files (.`rst`) inside `source/` directory. Do NOT modify any non documentation files (configurations, template, image, licensing, etc. files) without explicit confirmation.
By the way, are you happy for Copilot to modofy .rst files without confirmation?
There was a problem hiding this comment.
Thank you. Your suggestion is clearer. Regarding automatic .rst modifications: I'd recommend allowing Copilot to suggest changes to documentation files freely (that's its primary purpose here), but requiring confirmation for config/template/asset files.
.github/copilot-instructions.md
Outdated
| ensure compatibility with the [Ruff linter](https://docs.astral.sh). | ||
| - **RST Quality:** Adhere to [doc8 standards](https://github.com) for line | ||
| lengths and formatting. | ||
| - **RST Linting:** Follow rules enforced by `sphinx-lint`. |
There was a problem hiding this comment.
How can CoPilot do that? Can it run sphinx-lint?
There was a problem hiding this comment.
No, it cant execute the commands. As I understand Copilot uses those instructions as behavioural guidelines to mimic the results of those tools while it generates code or text for you. Please correct me if I am wrong.
.github/copilot-instructions.md
Outdated
| lengths and formatting. | ||
| - **RST Linting:** Follow rules enforced by `sphinx-lint`. | ||
| - **Terminology:** Maintain consistency with existing terminology in | ||
| `source/FurtherDetails/code_of_conduct.rst` and the project root. |
There was a problem hiding this comment.
The reference files does not contains any terminology but only the conduct that users should have.
Do you want copilot to have that code of conduct? Then I woul add them directly in this instructions.
Fo you want to refence a document with typical terminology, I would find a better document. You need to test to see if Compilot can follow referenced documentats in the instructutions.
There was a problem hiding this comment.
Well spotted - that file is about conduct, not terminology. I'll remove this misleading reference. If you have a glossary or terminology guide, I can adapt that instead.
.github/copilot-instructions.md
Outdated
| - **Python Snippets:** Always include type hints for any Python code snippets | ||
| provided within documentation blocks. | ||
| - **Validation:** If unsure of a Sphinx directive or configuration setting, | ||
| prioritise the standard [Sphinx Documentation](https://www.sphinx-doc.org) |
There was a problem hiding this comment.
Can Copilot reach an external web-site?
There was a problem hiding this comment.
No. But it relies on the pre-trained data (static context) in the underlying model. If the website contains information newer than the model's training data, Copilot will be cut-off from the new knowledge. I think the URL (acts as a strong keyword) provision here helps prioritise existing training data?
.github/copilot-instructions.md
Outdated
|
|
||
| ## Coding Standards | ||
| - Do not generate legal, licensing, or security policy text. | ||
| - Do not suggest direct commits to the `main` branch. |
There was a problem hiding this comment.
It took me a while to udnerstand that this is about "suggest" in the documentation, instead to the user or to copilot itself.
I would change it to
Do not suggest in the documentation the use of direct commit to the `main` branch.
There was a problem hiding this comment.
I like your rewording; it is much clearer - it's about documentation content, not Copilot's behaviour
.github/copilot-instructions.md
Outdated
| ## Coding Standards | ||
| - Do not generate legal, licensing, or security policy text. | ||
| - Do not suggest direct commits to the `main` branch. | ||
| - Do not include sensitive or proprietary data in examples. |
There was a problem hiding this comment.
What is sensitive and proprietary data? What is data?
What do you mean by examples? Code example?
Shouldn't MS make sure that code generated by Copilot can be used by us?
There was a problem hiding this comment.
Shouldn't MS make sure that code generated by Copilot can be used by us?
In principle yes. But I don't want to take any chance. Maybe I can be a bit more specific here:
"Do not include Met Office internal data, credentials, or system-specific details in code examples." ?
PR Summary
Following conversation on teams, I've updated instructions for the Copilot LLM engine using imperative language (Always, Never, Ensure, etc) and technical constraints to ensure the AI generates code and documentation that adheres to project's specific standards.
Code Reviewer: @mo-mguidolin
Code Quality Checklist
Code Review