Enhance Copilot instructions with Spack integration, formatting standards, and Git workflow#320
Conversation
|
@copilot Please adjust the instructions to ensure that when creating a branch for eventual pushing as a new upstream branch (e.g. for a PR), it should never have an upstream tracking branch even if created based on another branch. This eliminates the possibility of accidentally pushing commits to the base branch when pushing the new branch upstream. |
Added Git branch management guidance in commit 0e16acb. The instructions now explicitly state that new PR branches should never have upstream tracking, with rationale and example workflow included. |
|
@phlexbot format |
|
No automatic markdownlint fixes were necessary. |
|
No automatic cmake-format fixes were necessary. |
|
No automatic jsonnetfmt fixes were necessary. |
|
No automatic clang-format fixes were necessary. |
There was a problem hiding this comment.
Pull request overview
Updates the repository’s Copilot instructions to better reflect established local/CI workflows around Git branching, Spack-based environments, and language-specific formatting tools.
Changes:
- Adds Git branch management guidance to avoid accidentally pushing to the wrong branch.
- Documents Spack usage patterns (including
spack load <package>) and howsetup-env.shbehaves when Spack is/ isn’t available. - Documents formatting/linting tooling for C++, Python, and Jsonnet to align with repo/CI configuration.
- Add Spack package manager integration section with common usage patterns - Document C++ formatting (clang-format) and linting (clang-tidy) standards - Document Python formatting (ruff) and type checking (mypy) configurations - Document Jsonnet file formatting for workflow configurations - Provide guidance on dependency installation via both Spack and system packages Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
- Document that new PR branches should never have upstream tracking - Explain rationale: prevents accidentally pushing to base branch - Provide example workflow for creating and pushing feature branches - Address feedback from @greenc-FNAL on PR comment Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
- Change generic 'setup-env.sh' to explicit 'scripts/setup-env.sh' - Update dependency installation guidance to mention both repository and workspace-level paths - Addresses review feedback on line 165-166 Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
35e7529 to
c43e5f8
Compare
knoepfel
left a comment
There was a problem hiding this comment.
Thanks, @greenc-FNAL. Just a couple comments
.github/copilot-instructions.md
Outdated
| ### Jsonnet Files | ||
|
|
||
| - Jsonnet configuration files (`.jsonnet`) are used for Phlex workflow configurations | ||
| - Follow consistent formatting as enforced by CI checks |
There was a problem hiding this comment.
This instruction seems a little indirect--perhaps it's better to say Follow consistent formatting as enforced by jsonnetfmt?
There was a problem hiding this comment.
We certainly could; this is AI's own wording, supposedly to be the most clear to itself. Presumably at this point it will read the workflows and work out what needs to be done?
There was a problem hiding this comment.
I guess I was thinking in the context of someone's interactive development environment via (e.g.) Visual Studio Code. If they're using an agent in their Copilot chat, won't this file be used for instructions? If so, it seems less likely that someone's interactive session would have ready access to the workflow logs than if they had access to jsonnetfmt. But maybe I'm misunderstanding the purpose of these instructions.
There was a problem hiding this comment.
The instructions were generated by AI specifically to improve agent behavior. I will try to distinguish agent-specific instructions from those relevant for interactive use.
|
@copilot Per the conversations started by @knoepfel's review comments, please adjust |
- Add note clarifying environment setup applies to CI containers/devcontainers - Clarify Spack integration section is for CI/container environments - Update Jsonnet formatting to explicitly mention jsonnetfmt tool - Address feedback to distinguish instructions for different development contexts Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Updated in commit a2feb26. Added notes at the beginning of "Workspace Environment Setup" and "Spack Package Manager Integration" sections to clarify these instructions apply primarily to AI agents working in CI containers ( |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #320 +/- ##
==========================================
+ Coverage 80.19% 80.36% +0.17%
==========================================
Files 126 127 +1
Lines 3070 3102 +32
Branches 547 547
==========================================
+ Hits 2462 2493 +31
Misses 381 381
- Partials 227 228 +1
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Summary
Enhanced Copilot instructions with:
spack loadpattern for tools, with explicit path references toscripts/setup-env.shjsonnetfmt)phlex-ci,phlex-dev) or devcontainers versus human developers who may use different local setups (e.g., native macOS, Linux with system packages, or their own Spack configurations)All reviewer feedback has been addressed. The instructions now clearly indicate which guidance applies to specific development contexts, making them more useful for both automated agents and interactive human development.
<issue_title>✨ Set up Copilot instructions</issue_title>
><issue_description>Analyze and adjust instructions for this repository as documented in Best practices for Copilot coding agent in your repository.
> </issue_description>
>
><agent_instructions>When considering development environment, analyze the existing VSCode and codespace instructions in this repository, and be aware that applications made available via Spack can be brought into the current environment with
spack loadwhen required</agent_instructions>>
> ## Comments on the Issue (you are @copilot in this section)
>
>
>
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.