-
Notifications
You must be signed in to change notification settings - Fork 414
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
Fix Electron linting errors #929
Changes from 12 commits
80b00fc
7068eba
a212b8d
8b37ec6
094c3b8
aeb43ff
d39bd9d
33d5834
e4dd7f8
9d7f3f3
9f1d597
8947f9a
0edf895
6a3a3b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,30 +1,9 @@ | ||
/node_modules | ||
/LBRY-darwin-x64 | ||
/dist | ||
|
||
/src/main/dist | ||
/src/main/locales | ||
/src/main/node_modules | ||
/src/renderer/dist | ||
/build/venv | ||
/build/daemon.ver | ||
/lbry-app-venv | ||
/lbry-app | ||
/lbry-venv | ||
/build/venv | ||
*.pyc | ||
/static/daemon/lbrynet* | ||
/static/locales | ||
/daemon/build | ||
/daemon/venv | ||
/daemon/requirements.txt | ||
/.idea | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is here because many of us use JetBrains, which creates that dir There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's better to keep project related ignore rules into the project |
||
|
||
*.pyc | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this has to be here while upload_assets.py is still in use. if you want to rewrite that in JS instead, then we can drop python dependency completely, which would be sweet! |
||
*.iml | ||
.#* | ||
|
||
build/daemon.zip | ||
.vimrc | ||
|
||
package-lock.json | ||
|
||
.DS_Store |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,12 +31,11 @@ development and testing purposes. | |
* [Git](https://git-scm.com/downloads) | ||
* [Node.js](https://nodejs.org/en/download/) | ||
* [Yarn](https://yarnpkg.com/en/docs/install) | ||
* `yarn --add-python-to-path install --global --production windows-build-tools` (Windows only) | ||
|
||
### One-time Setup | ||
|
||
1. Clone this repo | ||
2. `DEPS=true ./build.sh` | ||
2. `./build.sh` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why did you get rid of DEPS=true? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no need anymore for installing dependencies from the build script. The user should install the prerequisites listed in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it would be much better if users ran a script to install dependencies instead of following a README. that way if the dependencies change, no one has to remember to update the README |
||
|
||
This will download and install the LBRY app and its dependencies, including | ||
[the LBRY daemon](https://github.com/lbryio/lbry) and command line utilities like `node` and `yarn`. | ||
|
@@ -61,40 +60,10 @@ The app can be run from the sources using the following command: | |
|
||
### On Windows | ||
|
||
#### Windows Dependency | ||
|
||
1. Download and install `git` from <a href="https://git-for-windows.github.io/">github.io<a> | ||
(configure to use command prompt integration) | ||
2. Download and install `npm` and `node` from | ||
<a href="https://nodejs.org/en/download/current/">nodejs.org<a> | ||
3. Download and install `python 2.7` from | ||
<a href="https://www.python.org/downloads/windows/">python.org</a> | ||
4. Download and Install `Microsoft Visual C++ Compiler for Python 2.7` from | ||
<a href="https://www.microsoft.com/en-us/download/confirmation.aspx?id=44266">Microsoft<a> | ||
5. Download and install `.NET Framework 2.0 Software Development Kit (SDK) (x64)` from | ||
<a href="https://www.microsoft.com/en-gb/download/details.aspx?id=15354">Microsoft<a> (may need | ||
to extract setup.exe and install manually by running install.exe as Administrator) | ||
|
||
#### One-time Setup | ||
|
||
1. Open a command prompt as administrator and run the following: | ||
|
||
``` | ||
npm install --global --production windows-build-tools | ||
exit | ||
``` | ||
|
||
2. Open a command prompt in the root of the project and run the following: | ||
|
||
``` | ||
python -m pip install -r build\requirements.txt | ||
npm install -g yarn | ||
yarn install | ||
yarn build | ||
``` | ||
|
||
3. Download the lbry daemon and CLI [binaries](https://github.com/lbryio/lbry/releases) and place | ||
them in `static\daemon`. | ||
Download the lbry daemon and CLI [binaries](https://github.com/lbryio/lbry/releases) and place them | ||
in `static\daemon`. | ||
|
||
### Build | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
{ | ||
"appId": "io.lbry.LBRY", | ||
"productName": "LBRY", | ||
"mac": { | ||
"category": "public.app-category.entertainment" | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
{ | ||
"name": "LBRY", | ||
"name": "lbry-app", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why did this change? will this affect the .deb package that's built? if the name changes, i think ubuntu will think its a different package... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed the name to follow the norms. I'm not sure how exactly it'll behave in Ubuntu since I didn't test on it, but it should normally use the I'll test it to be sure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Debian does identify the app as a new package and doesn't replace the one with the previous name. I think we should wait for the AppImage switch, which should come with the auto-update PR, before applying this change and then request Linux users that they uninstall their previous install. |
||
"version": "0.19.4", | ||
"description": "A browser for the LBRY network, a digital marketplace controlled by its users.", | ||
"homepage": "https://lbry.io/", | ||
|
@@ -38,7 +38,7 @@ | |
"install": "^0.10.2", | ||
"jayson": "^2.0.2", | ||
"jshashes": "^1.0.7", | ||
"keytar": "^4.0.3", | ||
"keytar-prebuild": "^4.0.4", | ||
"localforage": "^1.5.0", | ||
"npm": "^5.5.1", | ||
"qrcode.react": "^0.7.2", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i believe build/venv is still used (by build.sh)