Skip to content

Feature/standalone neutralino#9201

Draft
eliandoran wants to merge 9 commits intostandalonefrom
feature/standalone_neutralino
Draft

Feature/standalone neutralino#9201
eliandoran wants to merge 9 commits intostandalonefrom
feature/standalone_neutralino

Conversation

@eliandoran
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new standalone desktop application package for Trilium Notes using Neutralinojs. The changes include the addition of configuration files, build and development scripts, and basic project documentation. Feedback identifies several necessary updates to transition from the project template, such as aligning the package.json license with the MIT license file, providing a package description, and updating the applicationId and global variables in the Neutralino configuration. Additionally, a placeholder link in the README needs to be corrected, and an unnecessary async keyword should be removed from the build script.

},
"keywords": [],
"author": "",
"license": "ISC",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The license field is ISC, but the LICENSE file in this directory specifies the MIT License. To avoid confusion and potential legal issues, the license field in package.json should be consistent with the LICENSE file.

Suggested change
"license": "ISC",
"license": "MIT",


## Contributors

[![Contributors](https://contrib.rocks/image?repo=neutralinojs/neutralinojs-minimal)](https://github.com/neutralinojs/neutralinojs-minimal/graphs/contributors)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The contributor link points to neutralinojs/neutralinojs-minimal. This appears to be a leftover from a template. Please update this link to point to the correct repository for this project to properly credit its contributors.

@@ -0,0 +1,83 @@
{
"$schema": "https://raw.githubusercontent.com/neutralinojs/neutralinojs/main/schemas/neutralino.config.schema.json",
"applicationId": "js.neutralino.sample",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The applicationId is set to a sample value. It should be updated to a unique identifier for this application, for example io.github.triliumnext.desktop.

Suggested change
"applicationId": "js.neutralino.sample",
"applicationId": "io.github.triliumnext.desktop",

Comment on lines +21 to +32
"globalVariables": {
"TEST1": "Hello",
"TEST2": [
2,
4,
5
],
"TEST3": {
"value1": 10,
"value2": {}
}
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The globalVariables section contains sample data. These test variables should be removed if they are not used by the application.

  "globalVariables": {}

{
"name": "@triliumnext/standalone-desktop",
"version": "1.0.0",
"description": "",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The description field is empty. It's good practice to provide a brief description of the package to explain its purpose.

Suggested change
"description": "",
"description": "Standalone desktop application for Trilium Notes based on Neutralinojs",


const build = new BuildHelper("apps/standalone-desktop");

async function main() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The main function is declared as async but does not use any await expressions. The async keyword is unnecessary here and can be removed for better code clarity.

Suggested change
async function main() {
function main() {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant