Skip to content

fennel-ls: init at 0.1.0#281653

Merged
h7x4 merged 2 commits intoNixOS:masterfrom
yisraeldov:fennel-ls
Jan 27, 2024
Merged

fennel-ls: init at 0.1.0#281653
h7x4 merged 2 commits intoNixOS:masterfrom
yisraeldov:fennel-ls

Conversation

@yisraeldov
Copy link
Contributor

@yisraeldov yisraeldov commented Jan 17, 2024

Description of changes

Add package fennel-ls

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.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.

Add a 👍 reaction to pull requests you find important.

h7x4
h7x4 requested changes Jan 18, 2024
View reviewed changes
Copy link
Member

@h7x4 h7x4 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Here's a few suggestions!

Also, I think you need to rename the commit to fennel-ls: init at 0.1.0, to be consistent with the commit conventions

@yisraeldov yisraeldov requested a review from h7x4 January 18, 2024 01:18
@lafrenierejm
Copy link
Contributor

lafrenierejm commented Jan 20, 2024

Also, I think you need to rename the commit to fennel-ls: init at 0.1.0, to be consistent with the commit conventions

@yisraeldov Maybe also rename this PR's title to "fennel-ls: init at 0.1.0"?

@yisraeldov yisraeldov changed the title add fennel-ls package fennel-ls: init at 0.1.0 Jan 21, 2024
h7x4
h7x4 reviewed Jan 21, 2024
View reviewed changes
@h7x4
Copy link
Member

h7x4 commented Jan 21, 2024 •
edited
Loading

Could you move the commits around and rebase a bit to make it look like this? It's important that you add yourself to the maintainer list in the commit before the commit with the package.

maintainers: add yisraeldov
fennel-ls: init at 0.1.0

It might be a good idea to mark the PR as draft if you're unsure if you've rebased correctly. A wrong rebase can lead to pinging a lot of people.

@yisraeldov yisraeldov marked this pull request as draft January 21, 2024 15:14
@yisraeldov yisraeldov force-pushed the fennel-ls branch 2 times, most recently from 224866f to 85ab5f8 Compare January 21, 2024 15:27
@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Jan 21, 2024
@lafrenierejm
Copy link
Contributor

lafrenierejm commented Jan 21, 2024

@yisraeldov Looks like you accidentally pulled in an extra commit during the rebase.

@yisraeldov
maintainers: add yisraeldov
40b3614
@yisraeldov
fennel-ls: init at 0.1.0
8c746b5 
fennel-ls: init at 0.1.0

add fennel-ls package

Update pkgs/development/tools/language-servers/fennel-ls/default.nix

Co-authored-by: h7x4 <h7x4@nani.wtf>

Update pkgs/development/tools/language-servers/fennel-ls/default.nix

Co-authored-by: h7x4 <h7x4@nani.wtf>

Update pkgs/development/tools/language-servers/fennel-ls/default.nix

Co-authored-by: h7x4 <h7x4@nani.wtf>

Update pkgs/development/tools/language-servers/fennel-ls/default.nix

Co-authored-by: h7x4 <h7x4@nani.wtf>

Update pkgs/development/tools/language-servers/fennel-ls/default.nix

Co-authored-by: h7x4 <h7x4@nani.wtf>

Update pkgs/development/tools/language-servers/fennel-ls/default.nix

Co-authored-by: h7x4 <h7x4@nani.wtf>

fixes from code review

switch to 'by-name'
@yisraeldov
Copy link
Contributor Author

yisraeldov commented Jan 22, 2024

@yisraeldov Looks like you accidentally pulled in an extra commit during the rebase.

You are right, thanks, fixed

@h7x4
Copy link
Member

h7x4 commented Jan 22, 2024

Feel free to mark your PR as ready for review again, since the rebase went smooth 👍

@yisraeldov yisraeldov marked this pull request as ready for review January 22, 2024 07:41
@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jan 22, 2024
@h7x4 h7x4 requested a review from lafrenierejm January 23, 2024 08:59
Janik-Haag
Janik-Haag approved these changes Jan 27, 2024
View reviewed changes
Copy link
Member

@Janik-Haag Janik-Haag left a comment

Choose a reason for hiding this comment

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

LGTM

@delroth delroth removed the 12.approvals: 1 This PR was reviewed and approved by one person. label Jan 27, 2024
@delroth delroth added the 12.approvals: 2 This PR was reviewed and approved by two persons. label Jan 27, 2024
@h7x4 h7x4 merged commit ce5a607 into NixOS:master Jan 27, 2024
@yisraeldov yisraeldov mentioned this pull request May 5, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@h7x4 h7x4 h7x4 approved these changes

@piegamesde piegamesde Awaiting requested review from piegamesde

@lafrenierejm lafrenierejm Awaiting requested review from lafrenierejm

+1 more reviewer

@Janik-Haag Janik-Haag Janik-Haag approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@yisraeldov @lafrenierejm @h7x4 @Janik-Haag @delroth