Replace hardcoded PR/MR labels with dynamic forge-based labels #11449
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
- Refactor: rename GENERATE_MESSAGES to generateMessages for reactive clarity - Update clipboard messages and context menu labels to use dynamic forge labels - Update modal titles and CTA labels to use dynamic forge unit names Co-authored-by: Byron <63622+Byron@users.noreply.github.com>
Byron
force-pushed
the
copilot/dynamic-label-generation-mr
branch
from
8c29e0d to
f3c1034
Compare
|
Once again, this looks like a win. How does it look to you @Caleb-T-Owens? |
| {#snippet title()} | ||
| Auto-fill PR descriptions from commit | ||
| Auto-fill {prUnit?.abbr ?? 'PR'} descriptions from commit | ||
| {/snippet} | ||
| {#snippet caption()} | ||
| When creating a pull request for a branch with just one commit, automatically use that commit's | ||
| message as the PR title and description. |
There was a problem hiding this comment.
It doesn't make sense to dynamically switch here since it's in the global settings.
If we want to be "inclusive" about this one, we should say "PR/MR" here.
There was a problem hiding this comment.
Fixed in 1cf4d11. Changed to static "PR/MR" text since this is in global settings and not project-specific.
Caleb-T-Owens
left a comment
Caleb-T-Owens
left a comment
There was a problem hiding this comment.
The global settings either needs reverted, or changed to be statically "PR/MR", but the rest seems fine.
I'm not in love with all the duplicated defaults everywhere though.
I wonder if the forge factory should have it's own getters for unit, name, & friends that have the defaults there. That way we can cut down on the amount of fallbacks everywhere
|
Thanks for taking a look. Let's see if Copilot can see this PR through tot he end. @copilot Address the following comment:
|
Co-authored-by: Byron <63622+Byron@users.noreply.github.com>
Co-authored-by: Byron <63622+Byron@users.noreply.github.com>
Both addressed:
Refactored 7 components to use these getters, eliminating duplicated |
|
Alright, it really does seem like it addressed all concerns. I ran it and the most obvious place to see what changed really does make a more 'native' impression. top: nightly, bottom: dev
|
Strangely enough, this wasn't flagged when running `pnpm begood` locally. CI found it though, and it's definitely a real issue. It's unclear to me how that's possible.
Byron
force-pushed
the
copilot/dynamic-label-generation-mr
branch
from
74ff532 to
47d45db
Compare
🧢 Changes
GitLab users were seeing "PR" (Pull Request) terminology throughout the UI instead of "MR" (Merge Request). All hardcoded labels have been replaced with dynamic references to the forge's unit configuration.
Updated components:
Implementation:
Added centralized getters to
DefaultForgeFactoryto reduce code duplication:reviewUnitAbbr: Returns unit abbreviation with fallback to 'PR'reviewUnitName: Returns unit name with fallback to 'Pull request'reviewUnitSymbol: Returns unit symbol with fallback to '#'Each forge service defines
unit: { name, abbr, symbol }:{ name: 'Pull request', abbr: 'PR', symbol: '#' }{ name: 'Merge request', abbr: 'MR', symbol: '!' }Components now access these via
forge.reviewUnitAbbrinstead of duplicatedprService?.unit.abbr ?? 'PR'patterns throughout the codebase.Files modified:
☕️ Reasoning
Users expect UI terminology to match their forge. Hardcoded "PR" labels confused GitLab users and made the interface feel GitHub-centric. By adding centralized getters with fallback defaults, the code is now more maintainable with a single source of truth for default values.
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.