feat: changelog - formatted links, more git providers & copy markdown#2983
feat: changelog - formatted links, more git providers & copy markdown#2983WilcoSp wants to merge 303 commits into
Conversation
…ble to the version that was selected
…nd idk how it should be now
added scroll margin classes ensuring to navigate to hash if present
… back releases title's will now use slugify instead of encodeUri to fix issues with encoding
given changelogCard now also host added tests to ensure that email & package@version isn't formatted to a git link
…ithub.com:WilcoSp/npmx.dev into changelog/format-link+git-providers+copy-markdown
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shared/utils/git-providers.ts (1)
291-291: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType
NEED_HOSTasProviderId[].Currently inferred as
string[], so invalid or misspelt provider IDs would not be caught at compile time.As per coding guidelines: "Ensure you write strictly type-safe code".
♻️ Proposed change
-export const NEED_HOST = ['gitlab', 'gitea', 'forgejo', 'radicle'] +export const NEED_HOST: ProviderId[] = ['gitlab', 'gitea', 'forgejo', 'radicle']🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/utils/git-providers.ts` at line 291, Type the NEED_HOST constant explicitly as ProviderId[] so its provider entries are compile-time validated. Preserve the existing provider values and locate the change at the NEED_HOST declaration.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shared/utils/git-providers.ts`:
- Around line 305-311: Update the provider parsing logic around the discarded
NEED_HOST.includes(provider.id) expression to retain its boolean result, then
use that result when assigning host in the returned object. Ensure host is
populated only for providers listed in NEED_HOST and remains undefined for all
others.
---
Nitpick comments:
In `@shared/utils/git-providers.ts`:
- Line 291: Type the NEED_HOST constant explicitly as ProviderId[] so its
provider entries are compile-time validated. Preserve the existing provider
values and locate the change at the NEED_HOST declaration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 13f42098-7fdb-46aa-aa4c-f442d909cdaa
📒 Files selected for processing (9)
app/components/Changelog/Card.vueapp/components/Changelog/Releases.vueserver/api/changelog/md/[provider]/[owner]/[repo]/[...path].get.tsserver/api/changelog/releases/[provider]/[owner]/[repo].get.tsserver/api/changelog/releases/[provider]/[owner]/[repo]/raw/[tag].get.tsserver/utils/changelog/validateHost.tsshared/utils/constants.tsshared/utils/git-providers.tstest/unit/server/utils/changelog/markdown.spec.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- app/components/Changelog/Releases.vue
- app/components/Changelog/Card.vue
- server/api/changelog/releases/[provider]/[owner]/[repo]/raw/[tag].get.ts
- server/api/changelog/md/[provider]/[owner]/[repo]/[...path].get.ts
- server/api/changelog/releases/[provider]/[owner]/[repo].get.ts
- test/unit/server/utils/changelog/markdown.spec.ts
fixing the validation for host.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/unit/server/utils/changelog/validateHost.spec.ts`:
- Line 52: Correct the describe block title in the host validation tests by
changing “know host” to “known host”; leave the test behavior unchanged.
- Line 46: Correct the typo in the test description for the “require radicle to
have a host” case by changing “shoud” to “should.”
- Around line 179-182: Correct the test description in the radicle host
validation test so it states that gitea.com should be rejected or disallowed.
Keep the existing createSchema('radicle') setup and toBeFalsy assertion
unchanged.
- Around line 19-27: Update the second test description in the changelog host
validation tests to say it allows tangled without a host, while leaving its
createSchema('tangled') setup and assertions unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 700712cc-2f91-4022-a66a-ee0ede851d79
📒 Files selected for processing (5)
server/utils/changelog/detectChangelog.tsserver/utils/changelog/markdown.tsserver/utils/changelog/validateHost.tsshared/utils/constants.tstest/unit/server/utils/changelog/validateHost.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- server/utils/changelog/validateHost.ts
- shared/utils/constants.ts
- server/utils/changelog/detectChangelog.ts
- server/utils/changelog/markdown.ts
|
@coderabbitai is there anything left that should be addressed in this pr? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
gh pr view 2983 --repo npmx-dev/npmx.dev --json reviews,comments -q '.reviews[] | {author: .author.login, body: .body, state: .state}' | head -200
echo "---REVIEW COMMENTS---"
gh api repos/npmx-dev/npmx.dev/pulls/2983/comments --paginate | jq -r '.[] | select(.user.login=="coderabbitai") | "ID:\(.id) PATH:\(.path) LINE:\(.line) BODY:\(.body[0:200])"'Length of output: 23556 🏁 Script executed: #!/bin/bash
gh api graphql -f query='
query {
repository(owner: "npmx-dev", name: "npmx.dev") {
pullRequest(number: 2983) {
reviewThreads(first: 100) {
nodes {
isResolved
isOutdated
path
line
comments(first: 1) {
nodes {
author { login }
body
}
}
}
}
}
}
}' --jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.comments.nodes[0].author.login=="coderabbitai") | {path, line, isResolved, isOutdated, body: .comments.nodes[0].body[0:150]}'Length of output: 5640 All review threads from CodeRabbit are resolved. Nothing outstanding remains from my automated review.
A couple of earlier nitpicks (non-blocking, lower priority) that I don't have a resolved/unresolved thread confirmation for, in case you want to double check they were addressed:
If those are already handled, feel free to ignore this. Otherwise, this PR looks ready from my side. |
adding typing for NEED_HOST
|
I've just done the last bits that needed to be addressed, so for right now I do think this pr is complete and really ready for review. |
before I review, are there any parts to this PR that can be split up into multiple PRs? One indicator to me is that the PR title contains multiple distinct features - also see above |
|
no, these features can't be split off to separate PRs as during development they're made to work together, mostly with the multiple git providers support. |
| }) | ||
|
|
||
| describe('TabRoot + TabList + TabItem + TabPanel', () => { | ||
| function createTabsFixture(modelValue: string, idPrefix: string) { |
There was a problem hiding this comment.
there's a lint warning on this line
| @@ -0,0 +1,15 @@ | |||
| import { NEED_HOST, type ProviderId } from '~~/shared/utils/git-providers' | |||
| import { custom, optional } from 'valibot' | |||
There was a problem hiding this comment.
can you import this as import * as v from 'valibot' for consistency
| } | ||
| } | ||
|
|
||
| // source https://raw.githubusercontent.com/potch/sowpods/refs/heads/master/SOWPODS.txt and filtered with /^[a-f]{6,40}$/i |
| // return text | ||
|
|
There was a problem hiding this comment.
| // return text |
| ForgejoReleaseCollectionSchema, | ||
| GitlabReleaseCollectionSchema, | ||
| } from '~~/shared/schemas/changelog/release' | ||
| import { parse } from 'valibot' |
There was a problem hiding this comment.
v.parse (valibot is already imported also)
| : []), | ||
| ]) | ||
|
|
||
| //fetch README file as Markdown to copy |
There was a problem hiding this comment.
| //fetch README file as Markdown to copy | |
| // fetch README file as Markdown to copy |
| </script> | ||
| <template> |
There was a problem hiding this comment.
| </script> | |
| <template> | |
| </script> | |
| <template> |
preview
🔗 Linked issue
🧭 Context
📚 Description
added providers:
#, will in the future change due to tangled not having shorthands yet)(the following aren't required for this pr, just nice to have and are here if review takes long)
Adding support to format links to the git providers (same repo only):
Added button to allow copying the raw markdown
I did only use AI for some of the regular expressions
this pr continued from #2717 which continued from the pr before, that's why there are so many commits