Resolve Obsidian review findings - #1
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses Obsidian Community automated review findings by modernizing the plugin to Obsidian 1.13 APIs, tightening type-safety around frontmatter/URL handling, improving UI compatibility patterns, and hardening release/CI tooling (ESLint + build provenance) for the 0.4.1 release.
Changes:
- Migrates the settings tab to Obsidian 1.13 searchable setting definitions and bumps
minAppVersionto 1.13.0 (release version 0.4.1). - Replaces deprecated UI/async patterns (specialized
createDiv/createSpanhelpers;void-handled async callbacks) and improves type-safety in tag/frontmatter operations. - Adds official Obsidian ESLint flat config and GitHub provenance attestations in the release workflow.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
vitest.config.ts |
Adds a Node test setup file for consistent globals used by runtime code. |
versions.json |
Updates plugin-to-min-Obsidian version mapping for 0.4.1. |
tests/setup.ts |
Adds a minimal window shim for Node-based tests. |
src/tag-registry.ts |
Tightens frontmatter access typing and safely reads cached frontmatter properties. |
src/tag-manager-modal.ts |
Updates UI helpers and wraps async handlers to avoid unhandled promises. |
src/suggestion-modal.ts |
Updates UI helpers to Obsidian-preferred element creation methods. |
src/siliconflow.ts |
Uses window.URL/window.setTimeout APIs for better popout-window/DOM consistency. |
src/settings.ts |
Migrates settings UI to Obsidian 1.13 searchable setting definitions and refactors actions into helpers. |
README.md |
Adds an English overview and expands Vault access/privacy disclosure; updates dev/release notes. |
README_EN.md |
Expands Vault access disclosure; updates dev/release notes. |
package.json |
Bumps version to 0.4.1, removes unused dependency, adds ESLint tooling and lint script. |
manifest.json |
Bumps version to 0.4.1 and raises minimum Obsidian version to 1.13.0. |
eslint.config.mjs |
Introduces Obsidian-recommended ESLint flat config and TS project parsing. |
CHANGELOG.md |
Adds 0.4.1 release notes describing the review-driven changes. |
.github/workflows/release.yml |
Adds build provenance attestation and narrows published release assets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
+20
| import { defineConfig } from "eslint/config"; | ||
| import obsidianmd from "eslint-plugin-obsidianmd"; | ||
| import tseslint from "typescript-eslint"; | ||
|
|
||
| export default defineConfig([ | ||
| { | ||
| ignores: ["main.js", "release/**", "coverage/**", ".tmp/**", "node_modules/**"], | ||
| }, | ||
| ...obsidianmd.configs.recommended, | ||
| { | ||
| files: ["src/**/*.ts"], | ||
| languageOptions: { | ||
| parser: tseslint.parser, | ||
| parserOptions: { | ||
| project: "./tsconfig.json", | ||
| tsconfigRootDir: import.meta.dirname, | ||
| }, | ||
| }, | ||
| }, | ||
| ]); |
| }, | ||
| "scripts": { | ||
| "dev": "node esbuild.config.mjs", | ||
| "lint": "eslint \"src/**/*.ts\" package.json", |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
builtin-modulesdependencyWhy
The Obsidian Community automated review reported one source error and several compatibility, safety, documentation, dependency, and release recommendations for 0.4.0. This change addresses every actionable source and release finding. Vault enumeration remains because it is required for user-triggered tag import, usage counts, and Vault-wide rename/delete operations; the behavior is now explicitly disclosed.
Validation
eslint-plugin-obsidianmdrules: no warnings or errors