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

Linux Love and Care: Major rewrite which adds support for Linux Native, Flatpaks, Snaps and refactors most of the code #282

Open
wants to merge 9 commits into
base: development
Choose a base branch
from
Open
31 changes: 18 additions & 13 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"productName": "BetterDiscord Installer",
"description": "A simple standalone program which automates the installation, removal and maintenance of BetterDiscord.",
"author": "BetterDiscord",
"version": "1.1.1",
"version": "1.2.0",
Arcitec marked this conversation as resolved.
Show resolved Hide resolved
"license": "MIT",
"scripts": {
"dev": "electron-webpack dev",
Expand All @@ -15,25 +15,30 @@
"dist:dir": "yarn dist --dir -c.compression=store -c.mac.identity=null"
},
"dependencies": {
"source-map-support": "^0.5.16"
"source-map-support": "^0.5.21"
},
"//": [
Copy link
Member

Choose a reason for hiding this comment

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

Remove

Copy link
Author

@Arcitec Arcitec Sep 2, 2022

Choose a reason for hiding this comment

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

Are you absolutely 100% sure about that? This is just a comment. It does nothing except tell all contributors: "Don't attempt to update the Electron version unless you also do the huge job of rewriting the entire application to stop using remote. calls." It's a good thing to have this comment, since people who see the ancient Electron 13 version will be immediately tempted to upgrade it. But they can't do that. The comment tells them that, and lets them know what must change if they want to port it to a newer version of Electron. I'll remove it if you are totally sure though.

Copy link
Member

Choose a reason for hiding this comment

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

Yes I'm 100% certain about this, developer comments belong in better locations.

"IMPORTANT: Electron 13 is the final version that supports our 'remote' API.",
"Migrating to Electron 14+ will require a large rewrite of this application."
],
"devDependencies": {
"electron": "^9.4.0",
"electron-builder": "^22.4.1",
"electron": "^13.6.9",
"electron-builder": "^23.3.3",
"electron-webpack": "^2.8.2",
"eslint": "^7.21.0",
"eslint-plugin-svelte3": "^3.1.2",
"eslint": "^8.22.0",
"eslint-plugin-svelte3": "^4.0.0",
"find-process": "^1.4.7",
"focus-visible": "^5.2.0",
"phin": "^3.6.0",
"phin": "^3.6.1",
"rimraf": "^3.0.2",
"semver": "^7.3.5",
"svelte": "^3.38.2",
"svelte-loader": "^3.0.0",
"svelte-spa-router": "^3.1.0",
"semver": "^7.3.7",
"svelte": "^3.49.0",
"svelte-loader": "^3.1.3",
"svelte-spa-router": "^3.3.0",
"tmp": "^0.2.1",
"tree-kill": "^1.2.2",
"webpack": "~4.42.1",
"webpack-bundle-analyzer": "^4.4.0"
"webpack": "^4.46.0",
"webpack-bundle-analyzer": "^4.6.0"
},
"build": {
"appId": "app.betterdiscord.installer",
Expand Down
6 changes: 4 additions & 2 deletions src/main/update_installer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ import phin from "phin";
const semverGreaterThan = require("semver/functions/gt");
const {version} = require("../../package.json");


Copy link
Member

Choose a reason for hiding this comment

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

Please remove unnecessary additional newlines

Copy link
Author

@Arcitec Arcitec Sep 2, 2022

Choose a reason for hiding this comment

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

The dual newlines between functions was added for my own sanity as a contributor. Dual newlines between functions are very common in programming, to make large files easier to read, and to provide clear breaks between every function. Only having single line-breaks creates very suffocating, dense code which isn't very breathable or readable. Remember that your code is meant to be readable by every contributor, and this change helps that. The easier it is for a new contributor to read the code and figure it out, the better. There were already plenty of places in the original, unmodified code which used dual newlines. I would like to formally request that dual newlines between functions stays. If that is okay with you. :)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it is not up to contributors to determine the styleguide of a codebase. Plus doing both mass style and functional changes in one PR is out of scope imo.

const getJSON = phin.defaults({
method: "GET",
parse: "json",
headers: {"User-Agent": "BetterDiscord Installer"},
followRedirects: true
});


export default async function () {
const downloadUrl = "https://api.github.com/repos/BetterDiscord/Installer/releases";
console.info(`Better Discord Installer ${version}`);
Expand All @@ -24,7 +26,7 @@ export default async function () {

const result = await dialog.showMessageBox({
title: "New Installer Version Available",
message: `A new version of the BetterDiscord installer is available. Click "Download" to download the newest version.`,
message: `A new version of the BetterDiscord installer is available.\n\nClick "Download" to get the latest version.`,
buttons: ["Download", "Later"],
defaultId: 0,
cancelId: 1
Expand All @@ -34,7 +36,7 @@ export default async function () {
await shell.openExternal(latestRelease.html_url);
process.exit(0);
}

} else {
console.info(`The installer is up to date.`);
}
Expand Down
Loading