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

github-copilot-cli: init at 0.1.33 #228854

Merged
merged 3 commits into from
May 1, 2023
Merged

github-copilot-cli: init at 0.1.33 #228854

merged 3 commits into from
May 1, 2023

Conversation

malob
Copy link
Member

@malob malob commented Apr 29, 2023

Description of changes

Add new package.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@natsukium
Copy link
Member

"github-copilot-cli" was already packaged in #221090.

@wegank wegank marked this pull request as draft April 29, 2023 09:26
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 29, 2023
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 29, 2023
@ofborg ofborg bot requested review from Luflosi and winterqt April 29, 2023 22:37
@malob
Copy link
Member Author

malob commented Apr 30, 2023

Result of nixpkgs-review pr 228854 run on x86_64-darwin 1

1 package marked as broken and skipped:
  • fast-cli
2 packages failed to build:
  • epgstation
  • joplin
23 packages built:
  • balanceofsatoshis
  • bitwarden-cli
  • castnow
  • commitlint
  • create-cycle-app
  • github-copilot-cli
  • google-clasp
  • haskellPackages.tailwind
  • haskellPackages.tailwind.doc
  • image_optim
  • lighthouse
  • lv_img_conv
  • postcss-cli
  • theLoungePlugins.plugins.closepms
  • theLoungePlugins.themes.flat-blue
  • theLoungePlugins.themes.flat-dark
  • thelounge
  • uppy-companion
  • vimPlugins.coc-stylelint
  • vimPlugins.markdown-preview-nvim
  • vscode-extensions.rust-lang.rust-analyzer
  • wrangler
  • zx

@malob
Copy link
Member Author

malob commented Apr 30, 2023

Result of nixpkgs-review pr 228854 run on aarch64-darwin 1

1 package marked as broken and skipped:
  • fast-cli
2 packages failed to build:
  • epgstation
  • joplin
28 packages built:
  • balanceofsatoshis
  • bitwarden-cli
  • castnow
  • commitlint
  • create-cycle-app
  • emanote
  • emanote.data
  • github-copilot-cli
  • google-clasp
  • haskellPackages.emanote
  • haskellPackages.emanote.data
  • haskellPackages.emanote.doc
  • haskellPackages.tailwind
  • haskellPackages.tailwind.doc
  • image_optim
  • lighthouse
  • lv_img_conv
  • postcss-cli
  • theLoungePlugins.plugins.closepms
  • theLoungePlugins.themes.flat-blue
  • theLoungePlugins.themes.flat-dark
  • thelounge
  • uppy-companion
  • vimPlugins.coc-stylelint
  • vimPlugins.markdown-preview-nvim
  • vscode-extensions.rust-lang.rust-analyzer
  • wrangler
  • zx

@malob malob marked this pull request as ready for review April 30, 2023 00:48
@malob
Copy link
Member Author

malob commented Apr 30, 2023 •
edited
Loading

@ofborg build nodePackages.@githubnext/github-copilot-cli

@malob
Copy link
Member Author

malob commented Apr 30, 2023

@natsukium, thanks for catching that. Fixed.

@malob malob requested a review from wegank April 30, 2023 00:56
@malob malob requested a review from natsukium May 1, 2023 00:09
@wegank wegank merged commit 26bb09c into NixOS:master May 1, 2023
@malob malob deleted the add-github-copilot-cli branch May 1, 2023 04:39
@natsukium natsukium mentioned this pull request May 2, 2023
Open
dotlambda
dotlambda reviewed Sep 9, 2023
View reviewed changes
pkgs/tools/misc/github-copilot-cli/default.nix
meta = with lib; {
description = "A CLI experience for letting GitHub Copilot help you on the command line";
homepage = "https://githubnext.com/projects/copilot-cli/";
license = licenses.free;
Copy link
Member

Choose a reason for hiding this comment

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

Proof?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was an oversight on my part. Sorry about that. The package on NPM doesn't contain a license, and not knowing much about how licenses work, I didn't know what to select for the license, so I picked licenses.free, and hoped that other folks would point out if that was wrong. Might be a good idea for the nixpkgs manual to explicitly say in the lib.licenses.unfree that if a package does not have a license explicitly specified, the the package should be marked as unfree.

pkgs/tools/misc/github-copilot-cli/default.nix
platforms = platforms.all;
};
}

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

pkgs/tools/misc/github-copilot-cli/package-lock.json
Copy link
Member

Choose a reason for hiding this comment

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

Can we please stop vendoring thousands of lines?

Copy link
Member

Choose a reason for hiding this comment

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

@malob Please ask upstream to distribute a lock file and link to the issue so we can follow the conversation.

Copy link
Member Author

Choose a reason for hiding this comment

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

I would have loved to do this. Unfortunately, I wasn't able to find a Git repo I could contribute this change to upstream. As far as I'm aware this is the only location the source is hosted publicly: https://www.npmjs.com/package/@githubnext/github-copilot-cli?activeTab=code

@dotlambda dotlambda mentioned this pull request Sep 9, 2023
Closed
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotlambda dotlambda dotlambda left review comments

@natsukium natsukium natsukium approved these changes

@wegank wegank wegank approved these changes

@Luflosi Luflosi Awaiting requested review from Luflosi

@winterqt winterqt Awaiting requested review from winterqt

Assignees
No one assigned
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@malob @natsukium @dotlambda @wegank