Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add plugin: Instapaper #2960

Merged
merged 2 commits into from
Feb 6, 2024
Merged

Add plugin: Instapaper #2960

merged 2 commits into from
Feb 6, 2024

Conversation

jparise
Copy link
Contributor

@jparise jparise commented Jan 19, 2024

I am submitting a new Community Plugin

Repo URL

Link to my plugin: https://github.com/Instapaper/obsidian-instapaper

Release Checklist

  • I have tested the plugin on
    • Windows
    • macOS
    • Linux
    • Android (if applicable)
    • iOS (if applicable)
  • My GitHub release contains all required files
    • main.js
    • manifest.json
    • styles.css (optional)
  • GitHub release name matches the exact version number specified in my manifest.json (Note: Use the exact version number, don't include a prefix v)
  • The id in my manifest.json matches the id in the community-plugins.json file.
  • My README.md describes the plugin's purpose and provides clear usage instructions.
  • I have read the developer policies at https://docs.obsidian.md/Developer+policies, and have assessed my plugins's adherence to these policies.
  • I have read the tips in https://docs.obsidian.md/Plugins/Releasing/Plugin+guidelines and have self-reviewed my plugin to avoid these common pitfalls.
  • I have added a license in the LICENSE file.
  • My project respects and is compatible with the original license of any code from other plugins that I'm using.
    I have given proper attribution to these other projects in my README.md.

@joethei
Copy link
Collaborator

joethei commented Feb 2, 2024

id: 'instapaper-sync-notes',
Don't prefix the command id with the plugin id, that will happen internally automatically.

/ We delay the sync briefly to allow Obsidian time to finish loading;
use the following hook for that:

this.app.workspace.onLayoutReady(() => {
	// your code
});

import path from "path";
This import is available from a Node.js runtime, which means that this will throw errors on Mobile (that's a problem since you have isDesktopOnly marked as false in your manifest.json file.

path.join(folder, name + '.md')
Just join them with a normal slash /, in Obsidian all paths are using that format.

containerEl.createEl('h1', { text: 'Instapaper' });
Don't include a header with the plugin name in the settings

containerEl.createEl('h2', { text: 'Notes Sync' });
For section headings in settings use:

new Setting(containerEl).setName('name here').setHeading();

@joethei joethei added Changes requested Minor changes requested PR can be merged after some final changes have been requested and removed Ready for review labels Feb 2, 2024
@jparise
Copy link
Contributor Author

jparise commented Feb 2, 2024

Thanks for the review! I've applied your feedback in Instapaper/obsidian-instapaper#21.

id: 'instapaper-sync-notes', Don't prefix the command id with the plugin id, that will happen internally automatically.

The Command interface includes this comment:

    /**
     * Globally unique ID to identify this command.
     * @public
     */
    id: string;

... so perhaps consider noting the plugin-level namespacing there (vs "globally unique")?

@joethei joethei merged commit 5588055 into obsidianmd:master Feb 6, 2024
1 check passed
@jparise jparise deleted the patch-1 branch February 6, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes requested Minor changes requested PR can be merged after some final changes have been requested plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants