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.

Sign up for GitHub

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

Jump to bottom

Bump electron builder #257

Merged
merged 2 commits into from
Oct 5, 2016
Merged

Bump electron builder #257

merged 2 commits into from
Oct 5, 2016

Conversation

razzeee
Copy link
Contributor

@razzeee razzeee commented Aug 20, 2016

I've tested this on windows and the installer seems to perform as expected. And I couldn't find anything wrong with the program itself.

We might want to use this changed/new feature for macOS https://github.com/electron-userland/electron-builder/releases/tag/v5.6.0

@razzeee razzeee force-pushed the bump-electron-builder branch 2 times, most recently from be962ce to 72ca294 Compare August 20, 2016 21:06
@razzeee
Copy link
Contributor Author

razzeee commented Aug 20, 2016

Not sure why this is timing out on circleci. Already tried to push again, second time it hits the timeout.

@yuya-oc
Copy link
Contributor

yuya-oc commented Aug 21, 2016 •
edited
Loading

Somehow npm run installer takes too long time in the docker container. It might be better to consider the way to build without docker in CircleCI.

@razzeee razzeee force-pushed the bump-electron-builder branch from 72ca294 to 53d43ca Compare August 21, 2016 08:17
@razzeee razzeee changed the title Bump electron builder and electron-winstaller Bump electron builder Aug 21, 2016
@razzeee
Copy link
Contributor Author

razzeee commented Aug 21, 2016

Changed to only bump electron builder. As ncu (https://www.npmjs.com/package/npm-check-updates) doesn't show the winstaller update for some reason and I've read this electron/windows-installer#92 (comment)

@razzeee razzeee force-pushed the bump-electron-builder branch from 53d43ca to fe60797 Compare August 21, 2016 08:46
@razzeee
Copy link
Contributor Author

razzeee commented Aug 21, 2016

Same thing happening, I'll lower the dependency and try to see which version "breaks" it. Might take a few builds.

@razzeee razzeee force-pushed the bump-electron-builder branch 13 times, most recently from 69088bb to eb74425 Compare August 22, 2016 21:42
@razzeee razzeee mentioned this pull request Aug 22, 2016
Closed
@razzeee razzeee force-pushed the bump-electron-builder branch from eb74425 to 83f63b9 Compare September 2, 2016 08:38
@razzeee razzeee force-pushed the bump-electron-builder branch from 83f63b9 to a856ac9 Compare September 21, 2016 19:36
@razzeee razzeee force-pushed the bump-electron-builder branch 5 times, most recently from 4cbfffe to 51184a4 Compare October 1, 2016 19:40
@razzeee razzeee force-pushed the bump-electron-builder branch from 51184a4 to 10aef4e Compare October 1, 2016 19:59
@razzeee
Copy link
Contributor Author

razzeee commented Oct 1, 2016

Updated and also includes a small linux fix

yuya-oc
yuya-oc suggested changes Oct 3, 2016
View reviewed changes
Copy link
Contributor

@yuya-oc yuya-oc left a comment

Choose a reason for hiding this comment

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

We are glad if you tested before submitting. I think just bumping is not always good.

package.json
@@ -36,7 +40,7 @@
"chai": "^3.5.0",
"chai-as-promised": "^5.3.0",
"devtron": "^1.3.0",
"electron-builder": "5.2.1",
"electron-builder": "7.9.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

v7.9.0 has the bug, generated app can't start. You should use v7.10.2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That wasn't out when I bumped, will bump again later

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. So we are glad if you test before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry, but from my understanding, that's a deb package problem only. Do you really expect me to test every platform? I thought the PRs are there to check the code and get tests from every platform deemed important?
The artifact provided in https://circleci.com/gh/Razzeee/desktop/49#artifacts/containers/0 runs fine in windows. Is my understanding right, that electron-builder is used to generate every platform, even windows?

@yuya-oc yuya-oc added the Linux label Oct 3, 2016
@yuya-oc yuya-oc added this to the v3.5.0 milestone Oct 3, 2016
@razzeee razzeee force-pushed the bump-electron-builder branch from 10aef4e to f9d741a Compare October 3, 2016 20:46
@razzeee
Copy link
Contributor Author

razzeee commented Oct 3, 2016

Bumped version also seems to run fine on windows 10

@yuya-oc
Copy link
Contributor

yuya-oc commented Oct 3, 2016

For now, electron-builder is used only for .deb package. So I expected you had already tested that. I have tested v7.10.2 in this PR, so it's acceptable after new push.

@razzeee
Copy link
Contributor Author

razzeee commented Oct 5, 2016

Sorry, I think I really need to pick your brains about how everything is build and document it somewhere in the git/wiki

@yuya-oc
Copy link
Contributor

yuya-oc commented Oct 5, 2016

Sure.
But please understand it's very helpful for us to review if there is your analysis about what you are doing. Otherwise, I need to research everything from scrach whether I can merge the PR.

yuya-oc
yuya-oc approved these changes Oct 5, 2016
View reviewed changes
Copy link
Contributor

@yuya-oc yuya-oc left a comment

Choose a reason for hiding this comment

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

[email protected] was already tested in this PR.

@yuya-oc yuya-oc merged commit 2bc5793 into mattermost:master Oct 5, 2016
@razzeee razzeee deleted the bump-electron-builder branch October 5, 2016 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuya-oc yuya-oc yuya-oc approved these changes

Assignees
No one assigned
Labels
Linux
Projects
None yet
Milestone
v3.5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@razzeee @yuya-oc