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

Jump to bottom

nodePackages.clean-css: rename to clean-css-cli #84577

Closed
wants to merge 1 commit into from
Closed

nodePackages.clean-css: rename to clean-css-cli #84577

wants to merge 1 commit into from

Conversation

doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Apr 7, 2020 •
edited

Motivation for this change

Fix #82527 . (cc @justinlovinger ).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-review wip"
  • 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
  • Fits CONTRIBUTING.md.

@doronbehar
Copy link
Contributor Author

I got 2 PRs' commits mixed, the current set of commits should be relevant only for clean-css-cli now.

justinlovinger
justinlovinger approved these changes Apr 7, 2020
View reviewed changes
Copy link
Contributor

@justinlovinger justinlovinger left a comment

Choose a reason for hiding this comment

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

All files look good to me :)

@prusnak
Copy link
Member

prusnak commented May 30, 2020

Please rework your PR. It now has a merge conflict after PR #89184 has been merged

@doronbehar doronbehar force-pushed the clean-css-rename branch 2 times, most recently from d0848ee to caffce7 Compare May 31, 2020 05:15
@doronbehar
Copy link
Contributor Author

Fixed merged conflicts.

@doronbehar doronbehar closed this May 31, 2020
@doronbehar doronbehar reopened this May 31, 2020
@doronbehar
Copy link
Contributor Author

@GrahamcOfBorg build nodePackages.clean-css-cli

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-in-distress/3604/25

@veprbl veprbl self-assigned this Jun 5, 2020
@veprbl
Copy link
Member

veprbl commented Jun 7, 2020

We likely may need to do something with base64 hashes (e.g. apply #89520). Meanwhile, I think, the PR could stick to changing node-packages.json file only.

@doronbehar
Copy link
Contributor Author

@veprbl how often does someone run ./generate.sh for the node package set?

@veprbl
Copy link
Member

veprbl commented Jun 9, 2020

Landed as 9a47504

@veprbl how often does someone run ./generate.sh for the node package set?

If you are asking for a suggestions, I'm not sure what to tell you. While I was applying this PR and another one, some other upstream dependencies had already been bumped. The way it is now, any PR that regenerates node-packages.nix will pick up irrelevant changes that would always lead to merge conflicts. Perhaps, it would work a bit more predictably if node2nix could fetch some specific snapshot of the npm registry.

@veprbl veprbl closed this Jun 9, 2020
@veprbl veprbl removed their assignment Jun 9, 2020
@doronbehar doronbehar deleted the clean-css-rename branch March 2, 2023 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prusnak prusnak prusnak approved these changes

@justinlovinger justinlovinger justinlovinger approved these changes

Assignees
No one assigned
Labels
6.topic: nodejs 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

nodePackages.clean-css is the wrong package
5 participants
@doronbehar @prusnak @nixos-discourse @veprbl @justinlovinger