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

zoom-us: 2.0.91373.0502 -> 2.0.98253.0707, and minor cleanups #27892

Merged
merged 4 commits into from Aug 4, 2017
Merged

zoom-us: 2.0.91373.0502 -> 2.0.98253.0707, and minor cleanups #27892

merged 4 commits into from Aug 4, 2017

Conversation

jameysharp
Copy link
Contributor

Motivation for this change
  • Upgrade to newer upstream version;
  • Update homepage;
  • Only declare platform support for systems we have a source URL available for;
  • Generate a simpler wrapper by using makeWrapper directly instead of the wrapProgram helper.

As maintainer, could you review these patches @danbst?

Things done

Please check what applies. Note that these are not hard requirements but merely serve as information for reviewers.

  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@jameysharp
zoom-us: update homepage
0ce122f 
zoom.us uses HTTPS now.
@jameysharp
zoom-us: only support known platforms
598790f 
This package does not work on arbitrary Linux systems, only on platforms
for which upstream has provided prebuilt binaries. Fortunately, we have
a list of the platforms we know how to get binaries for: it's exactly
the ones in the `srcs` set.
@jameysharp
zoom-us: use makeWrapper instead of wrapProgram
e5e6d56 
Since the program to be wrapped is already in a different path than
$out/bin, we don't need the complicated dance that wrapProgram uses to
put the wrapper in the same location as the original program. Just tell
makeWrapper to put the wrapper in the final desired output location
instead.
@jameysharp
zoom-us: 2.0.91373.0502 -> 2.0.98253.0707
462e442
@mention-bot
Copy link

@jameysharp, thanks for your PR! By analyzing the history of the files in this pull request, we identified @danbst to be a potential reviewer.

@danbst
Copy link
Contributor

danbst commented Aug 3, 2017

Nice cleenup! Tested, it works. Out of additional benefits, google login now works.

I got crash on first start, but subsequent starts didn't crash. I guess I had something wrong in ~/.zoom

$ zoom-us                                                                                                                                                       
zoom started.                                                                                                                                                                                                   
[CZPClientLogMgr::LogClientEnvironment] [MacAddr: 84:16:F9:14:A3:A6][client: Linux][OS: NixOS 17.09.pre.1e1472e (Hummingbird)][Hardware: CPU Core:2 Frenquency:1.9 G Memory size:3391MB CPU Brand:AMD A8-7600 Radeon R7, 10 Compute Cores 4C+6G  ][Req ID: ]
Linux Client Version is 2.0.98253.0707
QSG_RENDER_LOOP is 
Graphics Card Info:: 
Zoom package arch is 64bit, runing OS arch is x86_64
AppIconMgr::systemDesktopName log Desktop Name: none+awesome                                                                                                                                                    
CSBConfUI::OnConfStatusChanged  UI_CMD_SHARE_READYSegmentation fault

@globin globin merged commit de76112 into NixOS:master Aug 4, 2017
@jameysharp jameysharp deleted the zoom-us branch August 4, 2017 01:40
@jameysharp
Copy link
Contributor Author

Thank you both! 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jameysharp @mention-bot @danbst @globin