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

Fix desktop build instructions #3237

Closed

Conversation

andreygursky
Copy link

@andreygursky andreygursky commented Feb 16, 2017

Fixes #2792 again after regression 3c91f90

@ara4n
Copy link
Member

ara4n commented Feb 19, 2017

@dbkr was there a reason you removed the npm run build step?

@dbkr
Copy link
Member

dbkr commented Feb 20, 2017

npm run build is included in the npm run dist step in the 'Building from source' bit, so you don't need to run both. Really it should be npm run build instead of npm run dist as the tarball is unnecessary. I'll make the instructions say this instead.

For future reference, please note the contributing guide at https://github.com/vector-im/riot-web/blob/master/CONTRIBUTING.rst: we need DCO signoff and PRs against the develop branch, (although for doc fixes like this one master may be acceptable).

dbkr added a commit that referenced this pull request Feb 20, 2017
Clarify that you only need to run build rather than dist to run
the electron app.

Hopefully satisfies: #3237
@dbkr dbkr closed this Feb 20, 2017
@andreygursky andreygursky deleted the fix-doc-build-desktop branch February 20, 2017 15:45
@andreygursky
Copy link
Author

Thanks for clarification.

The trade-off with users pulling master and developers forced to pull developer branch is not usual, thanks for pointing this out. Before committing I look at other commits and make mine according to this style. I haven't noticed other contributors signing their commits. Does it mean they could potentially every time bring the whole project down? E.g.: @aviraldg? Some commits contain only nickname: c1290fd, f10bc8e.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants