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

yakyak init at 1.5.4-beta-rolling and (patched) electron-packager init at 14.0.6 #69393

Closed
wants to merge 4 commits into from

Conversation

hlolli
Copy link
Member

@hlolli hlolli commented Sep 24, 2019

Motivation for this change

new package from #55289

Things done

the wildcards in this PR are two things, first is this patched wrapper around electron-packager, I'm sure I can reuse it for other electron packages in the future. It just prevents electron-packager from downloading the binaries and tells it to use the ones already provided from nixpkgs (so overriding the electron input would be the way to choose a different electron version).

The second wildcard is the hangupsjs, which had a terrible conflict in coffee-script. Using the version provided in the yakyak npm package causes compilation error, this syntax error is already a ticket here yakyak/hangupsjs#79 and when it's solved, I will bump and remove the hangusjs as a global nodePackage (and use it just as part of the yakyakBuldDeps).

  • [ x ] Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • [ x ] 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 nix-review --run "nix-review wip"
  • [ x ] Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • [ x ] Fits CONTRIBUTING.md.

@sir4ur0n
Copy link

Hi,

What's the next step for this PR please?

Cheers

@doronbehar
Copy link
Contributor

For an easy merge of this, I'd suggest not using node-packages-v10.{json,nix} directly, but use the approach taken by e.g remarkjs where a distinct set of packages is used solely for the main package. node-packages-v10.{json,nix} are updated frequently and it's easier to maintain these sets separately on a per package basis for these NodeJS / Atom / Electron / Whatever based packages.

I know this is Nixpkgs' approach is inconsistent but it was mentioned before that this approach is better and as you can see, it's easier to maintain..

@Enteee
Copy link
Contributor

Enteee commented Dec 5, 2019

I would suggest splitting this PR into one pull request per package.

@stale
Copy link

stale bot commented Jun 2, 2020

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse.
  3. Ask on the #nixos channel on irc.freenode.net.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 2, 2020
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 3, 2020
@mnajib
Copy link

mnajib commented Oct 14, 2020

still important to me

@doronbehar
Copy link
Contributor

Unsurprisingly, there are merge conflicts. You may find https://discourse.nixos.org/t/call-for-nodepackages-maintainers/9176/ relevant for this.

@stale
Copy link

stale bot commented Apr 18, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 18, 2021
@AleXoundOS
Copy link
Contributor

Still important for me.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 13, 2022
@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 8, 2023
@DamienCassou
Copy link
Contributor

If this PR is still important to anyone, could this person feel responsible for applying reviewer's suggestions and resolving conflicts?

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 12, 2023
@hlolli
Copy link
Member Author

hlolli commented Oct 12, 2023

it seems yakyak is mostly abandoned, I don't have interest in moving this forward, whoever's interested and wants to use the work I did thus far, feel free to open a new pr

@hlolli hlolli closed this Oct 12, 2023
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.

None yet

8 participants