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

skypeforlinux: 5.5.0.1 -> 8.10.76.2 #31129

Closed
wants to merge 1 commit into from
Closed

skypeforlinux: 5.5.0.1 -> 8.10.76.2 #31129

wants to merge 1 commit into from

Conversation

cruizh
Copy link
Contributor

@cruizh cruizh commented Nov 2, 2017 •
edited

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • 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.

@orivej
Copy link
Contributor

orivej commented Nov 2, 2017

  1. We do not merge to stable without first merging to master.
  2. We do not upgrade stable without good reason. Why do you want it upgraded? Skype 5 seems to work fine.
  3. Currently skypeforlinux 8 will require a lot of effort to maintain because they delete old minor versions as soon as they release new ones, and they change 8 much more often than 5.

@orivej
Copy link
Contributor

orivej commented Nov 2, 2017 •
edited

One thing we could definitely do is merge skypeforlinux 8 into master under a new attribute skypeforlinux8 if either its name is not skypeforlinux-${version}, or it is adjusted with lowPrio.

Mic92
Mic92 reviewed Nov 2, 2017
View reviewed changes
pkgs/applications/networking/instant-messengers/skypeforlinux/default.nix
@@ -4,7 +4,7 @@

let

version = "5.5.0.1";
version = "8.10.76.2";
Copy link
Member

Choose a reason for hiding this comment

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

I think it is time to add a note regarding 8.x is unstable. Every now and then somebody tries to update it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good.

However, 8.9.0.1 is now stable; it may outlive unstable versions in the 8.10.* series. Yet the feedback about it is hilarious:

With the old version of skypeforlinux i didn't had any problems, when I updated it today, it started, but for some reason, after a few clicks, it goes to blank.

After I downloaded the new version of Skype for Desktops I started getting advertisement popups. How can I stop getting them?

but how can I change my status ... I followed help but I cannot change the status ... no button

I am just amazed at this haphazard, purely random changes you made, instead of trying to restore at least part of useful functionality from 4.3.

Thank you Microsoft for continuing to work on Skype for Linux! I do appreciate the effort.
With that said, I must say that screen sharing is still a huge issue! At least I have a button for it now, but unfortunately, it doesn't work out of the box.

https://answers.microsoft.com/en-us/skype/forum/skype_linux-skype_startms-skype_installms/the-new-skype-for-desktop-is-here/dba5e41b-126a-4221-9e28-e1266d235689

Copy link
Member

Choose a reason for hiding this comment

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

Well something is always broken for somebody. Have you personal experience with this version?

Copy link
Contributor

@orivej orivej Nov 2, 2017 •
edited

Choose a reason for hiding this comment

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

Jokes aside, just a few days after the release Debian says that the blank screen affects multiple users, and Microsoft explains on the linked page that this happens on CPUs without SSSE3 and they are going to fix it and that in the meantime users should stay on 5.5. So adding skypeforlinux 8 as skypeforlinux8 is fine, but replacing 5.5 is not.

@cruizh
Copy link
Contributor Author

cruizh commented Nov 2, 2017

  1. We do not merge to stable without first merging to master.

Sorry, this was my first time submitting changes and I found the manual a bit confusing, suggesting I edit off the release branch (and then obviously rebasing to master is a mess). Furthermore, the CONTRUBTING document did not specify which branch to pull request. Still, point taken. This change (if wanted) should be merged to master instead.

  1. We do not upgrade stable without good reason. Why do you want it upgraded? Skype 5 seems to work fine.
  2. Currently skypeforlinux 8 will require a lot of effort to maintain because they delete old minor versions as soon as they release new ones, and they change 8 much more often than 5.

Fair enough.

One thing we could definitely do is merge skypeforlinux 8 into master under a new attribute skypeforlinux8 if either its name is not skypeforlinux-${version}, or it is adjusted with lowPrio.

This would be an interesting solution, but I feel I do not have the knowledge to properly discuss it.

@orivej
Copy link
Contributor

orivej commented Nov 2, 2017 •
edited

@cruizh Thank you for the feedback, we will improve CONTRIBUTING.

lowPrio is just a prefix for callPackage, you can find examples in all-packages.nix. It is needed so that users of skypeforlinux 5 will not upgrade to 8 with a nix-env -u (if both derivations have the same name — otherwise 8 will not be a candidate for update from 5). I believe it is best not to upgrade to skypeforlinux 8 for the time being.

@cruizh cruizh closed this Nov 3, 2017
@orivej orivej mentioned this pull request Nov 10, 2017
Merged
8 tasks
@orivej orivej mentioned this pull request Dec 14, 2017
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Mic92 left review comments

@orivej orivej orivej left review comments

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 0 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@cruizh @orivej @Mic92 @GrahamcOfBorg @Janik-Haag