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

Upgrade dependencies #906

Merged
merged 10 commits into from
Jan 11, 2019
Merged

Conversation

yuya-oc
Copy link
Contributor

@yuya-oc yuya-oc commented Dec 5, 2018

Before submitting, please confirm you've

Please provide the following information:

Summary
Upgrade dependencies.

After this PR, we will get ready to switch back to npm.

Issue link
#898

Test Cases

  • Apps should normally work.
  • "Save Image" context menu should work.

Additional Notes
https://circleci.com/gh/yuya-oc/desktop/1187#artifacts

@yuya-oc yuya-oc added the All Platforms null label Dec 5, 2018
@yuya-oc yuya-oc added this to the v4.3.0 milestone Dec 5, 2018
@yuya-oc
Copy link
Contributor Author

yuya-oc commented Dec 11, 2018

Looks good to me.

  • Windows 10 Pro 64-bit (zip, exe)
  • macOS 10.14.1 (zip, dmg)

@yuya-oc yuya-oc force-pushed the upgrade-dependencies branch from 135cae0 to 3bfb9d6 Compare December 25, 2018 14:31
@yuya-oc
Copy link
Contributor Author

yuya-oc commented Dec 25, 2018

Rebased against the latest master branch.

Artifacts: https://circleci.com/gh/yuya-oc/desktop/1211#artifacts

@yuya-oc yuya-oc mentioned this pull request Dec 25, 2018
11 tasks
@yuya-oc
Copy link
Contributor Author

yuya-oc commented Dec 27, 2018

Added Electron v4. As far as reading the release notes, I could not find critical changes for us (However v5 will break the app due to default webPreferences values).
https://electronjs.org/releases#4.0.0

Artifacts: https://circleci.com/gh/yuya-oc/desktop/1215#artifacts

@yuya-oc yuya-oc requested review from wget and crspeller December 27, 2018 16:04
Copy link
Collaborator

@wget wget left a comment

Choose a reason for hiding this comment

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

The build succeeded on my Windows 10 Pro x64 1809 dev machine.

The build succeeded on AppVeyor as well BUT only 1 test is failing. Could we please fix this before merging this pull request?

Here are the AppVeyor logs with the details of the failing test: https://ci.appveyor.com/project/wget/mattermost-desktop/builds/21360791

Also, for a reason I don't know (even if this is NOT a regression) I realized the GUI on Windows was blurry compared to the web ui on the left:

screenshot_20190103_154147

I checked by disabling the GPU rendering, but without any improvement. Are we aware of this issue? Is a dedicated bug report already open?

@yuya-oc yuya-oc force-pushed the upgrade-dependencies branch from e59ef9c to e62f16a Compare January 7, 2019 14:32
@yuya-oc
Copy link
Contributor Author

yuya-oc commented Jan 7, 2019

@wget Updated the branch. What about this?

For blurry rendering, I think it's known issue (#560, electron/electron#10025). The reason is that Electron's webview unexpectedly uses grayscale antialiasing. It looks like blurry.

@wget
Copy link
Collaborator

wget commented Jan 9, 2019

@yuya-oc Everything worked like expected. The migration from yarn to npm went smoothly. All tests passed.

Artefacts: https://ci.appveyor.com/project/wget/mattermost-desktop/builds/21505020

It's finally time to merge!

@crspeller crspeller merged commit 5abe83b into mattermost:master Jan 11, 2019
@yuya-oc yuya-oc deleted the upgrade-dependencies branch January 16, 2019 15:27
@yuya-oc yuya-oc mentioned this pull request Feb 18, 2019
3 tasks
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.

4 participants