Conversation
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Pull request overview
Adds an initial documentation entry point for the Community Toolkit Perl hosting integration, including nav placement and related data/assets in the frontend docs site.
Changes:
- Added a new Perl framework doc page and Perl icon asset.
- Added “Perl” to the Integrations sidebar under Frameworks & runtimes.
- Added the Perl package ID to the integrations name list and added new license title entries.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/frontend/src/data/thanks-license-titles.ts | Adds new license title entries for Perl-related assets (currently includes a TS syntax error). |
| src/frontend/src/data/aspire-integration-names.json | Adds CommunityToolkit.Aspire.Hosting.Perl to the integration package name list. |
| src/frontend/src/content/docs/integrations/frameworks/perl.mdx | Introduces a new Perl integration doc page (currently mostly placeholder content). |
| src/frontend/src/assets/icons/perl-096-1500.svg | Adds a Perl logo SVG used by the new doc page. |
| src/frontend/config/sidebar/integrations.topics.ts | Adds the Perl doc page to the integrations sidebar nav. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| dapr: 'Apache-2.0: https://github.com/dapr/dapr/blob/master/LICENSE', | ||
| hex1b: 'MIT: https://hex1b.dev', | ||
| asciinema: 'GPL-3.0: https://github.com/asciinema/asciinema/blob/develop/LICENSE', | ||
| perl-assets: 'CC-BY-4.0: https://github.com/metacpan/perl-assets?tab=CC-BY-4.0-1-ov-file#readme', |
There was a problem hiding this comment.
perl-assets is not a valid unquoted property name in a TS object literal (the hyphen makes this a syntax error), so this file won’t compile. Rename the key to a valid identifier (e.g., perlAssets) or quote it (e.g., 'perl-assets') and update any corresponding references.
| perl-assets: 'CC-BY-4.0: https://github.com/metacpan/perl-assets?tab=CC-BY-4.0-1-ov-file#readme', | |
| 'perl-assets': 'CC-BY-4.0: https://github.com/metacpan/perl-assets?tab=CC-BY-4.0-1-ov-file#readme', |
| "CommunityToolkit.Aspire.Hosting.Ollama", | ||
| "CommunityToolkit.Aspire.Hosting.OpenTelemetryCollector", | ||
| "CommunityToolkit.Aspire.Hosting.PapercutSmtp", | ||
| "CommunityToolkit.Aspire.Hosting.Perl", |
There was a problem hiding this comment.
CommunityToolkit.Aspire.Hosting.Perl is added to the integration name list, but there’s no corresponding entry in src/frontend/src/data/integration-docs.json mapping that package ID to /integrations/frameworks/perl/. Without that, the Integrations gallery won’t surface a docs link for this package.
| "CommunityToolkit.Aspire.Hosting.Perl", |
…e harness first. Also changed some of the feedback from the PR.
…fixing the thanks-license-titles.ts
| --- | ||
|
|
||
| ## Table of Contents | ||
|
|
||
| 1. [Quick Start](#quick-start) | ||
| 2. [Core Concepts](#core-concepts) | ||
| 3. [The `appDirectory` Parameter](#the-appdirectory-parameter) | ||
| 4. [WithLocalLib](#withlocallib) | ||
| 5. [Package Management](#package-management) | ||
| - [WithCpanMinus + WithPackage](#withcpanminus--withpackage) | ||
| - [WithCpanMinus + WithProjectDependencies](#withcpanminus--withprojectdependencies) | ||
| - [WithCarton + WithProjectDependencies](#withcarton--withprojectdependencies) | ||
| 6. [WithPerlbrewEnvironment](#withperlbrewenvironment) | ||
| 7. [Common Pitfalls](#common-pitfalls) | ||
| 8. [Reporting an Issue](#reporting-issues) | ||
|
|
||
| --- |
There was a problem hiding this comment.
We get this for free... not needed.
| --- | |
| ## Table of Contents | |
| 1. [Quick Start](#quick-start) | |
| 2. [Core Concepts](#core-concepts) | |
| 3. [The `appDirectory` Parameter](#the-appdirectory-parameter) | |
| 4. [WithLocalLib](#withlocallib) | |
| 5. [Package Management](#package-management) | |
| - [WithCpanMinus + WithPackage](#withcpanminus--withpackage) | |
| - [WithCpanMinus + WithProjectDependencies](#withcpanminus--withprojectdependencies) | |
| - [WithCarton + WithProjectDependencies](#withcarton--withprojectdependencies) | |
| 6. [WithPerlbrewEnvironment](#withperlbrewenvironment) | |
| 7. [Common Pitfalls](#common-pitfalls) | |
| 8. [Reporting an Issue](#reporting-issues) | |
| --- |
| builder.Build().Run(); | ||
| ``` | ||
|
|
||
| --- |
There was a problem hiding this comment.
Please remove these, they end up rendering as <hr />
| .WithLocalLib("local") // relative path — resolved against appDirectory | ||
| .WithLocalLib("/opt/lib") // rooted Unix-style path — used as-is | ||
| .WithLocalLib("C:\\perl-lib") // rooted Windows path — used as-is |
There was a problem hiding this comment.
Lingering code without proper indentation or context. Assume readers will copy code - this will not compile.
| > The default package manager is `cpan`, but it is automatically switched to `cpanm` when | ||
| > `WithProjectDependencies()` is called, since `cpan` does not support `--installdeps`. | ||
| > `WithLocalLib()` will also currently swap to `cpanm` because it wasn't clear to me at time of release how to integrate it with cpan. |
| 1. The integration looks for `cpanfile` in the working directory | ||
| 2. Runs `cpanm --installdeps --notest .` (with `--local-lib` if configured) | ||
| 3. All dependencies from the cpanfile are installed |
| Further reading: | ||
| - [CPAN::cpanfile reference](https://github.com/miyagawa/cpanfile/blob/master/README.md) | ||
|
|
||
| ## Reporting Issues |
There was a problem hiding this comment.
Use sentence casing for headings.
The perl integration has been pulled into main of the Community Toolkit, but I believe the package is not yet available.
I wanted to go ahead and submit this to see what else I need to do to add documentation for it when it does go live and get published.