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

elmPackages: add node packages #101484

Closed
wants to merge 1 commit into from
Closed

elmPackages: add node packages #101484

wants to merge 1 commit into from

Conversation

ursi
Copy link
Contributor

@ursi ursi commented Oct 23, 2020

Added:

  • elm-install
  • elm-git-install

Other node packages have also been updated via node2nix

Motivation for this change
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.

ehmry
ehmry approved these changes Nov 27, 2020
View reviewed changes
Copy link
Contributor

@ehmry ehmry left a comment

Choose a reason for hiding this comment

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

I don't know Elm but looks harmless to me.

@ursi
Copy link
Contributor Author

ursi commented Nov 28, 2020 •
edited

oof, the spacing is wrong

@ursi
Copy link
Contributor Author

ursi commented Nov 28, 2020

Okay how can I fix this?

@ehmry
Copy link
Contributor

ehmry commented Nov 28, 2020

Rebase on the current master and force push.

@ursi
elmPackages: add node packages
99a7db7 
Added:
- elm-install
- elm-git-install

Other node packages have also been updated via node2nix
@ursi
Copy link
Contributor Author

ursi commented Nov 28, 2020

@ehmry I'd like to not have this sit here so long that I have to do this again. Is there anything I can do to up the chances of getting it merged?

@stale
Copy link

stale bot commented Jul 21, 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 Jul 21, 2021
@turboMaCk turboMaCk self-requested a review December 8, 2021 22:33
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 8, 2021
@turboMaCk
Copy link
Member

I'm going to look into this PR

@turboMaCk turboMaCk mentioned this pull request Dec 11, 2021
Closed
10 tasks
turboMaCk
turboMaCk reviewed Dec 11, 2021
View reviewed changes
pkgs/development/compilers/elm/packages/node-packages.json
@@ -9,5 +9,7 @@
"elm-verify-examples",
"elm-xref",
"create-elm-app",
"elm-optimize-level-2"
"elm-optimize-level-2",
Copy link
Member

Choose a reason for hiding this comment

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

this package was added in #96594

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What exactly are you getting at here?

Copy link
Member

Choose a reason for hiding this comment

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

just cleaning up old PRs. This one went under the carpet since no committer spotted it. Since then elm-optimize-level-2 was added. This adds elm-git-install #150458. That means elm-install is the only package from this PR which is not merged. I wonder - do you still want to include it in nixpkgs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you still want to include it in nixpkgs?

I don't see myself writing elm any time soon, so I'm kinda indifferent about it at this point. But if I don't have to do much to get it in, then yeah, why not.

@turboMaCk turboMaCk mentioned this pull request Dec 12, 2021
Merged
13 tasks
@turboMaCk
Copy link
Member

I'm going to close this PR since it's outdated. Anyway @ursi feel free to ping me directly under your future PRs to nixpkgs. I might be able to get some of them in.

@turboMaCk turboMaCk closed this Dec 12, 2021
@ursi
Copy link
Contributor Author

ursi commented Dec 13, 2021

@turboMaCk thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@turboMaCk turboMaCk turboMaCk left review comments

@ehmry ehmry ehmry approved these changes

Assignees
No one assigned
Labels
2.status: merge conflict 8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ursi @ehmry @turboMaCk @SuperSandro2000