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

otree: init at 0.2.0 #316252

Closed
wants to merge 2 commits into from
Closed

otree: init at 0.2.0 #316252

wants to merge 2 commits into from

Conversation

0x61nas
Copy link
Member

@0x61nas 0x61nas commented May 31, 2024 •
edited
Loading

Description of changes

addition otree package, which is a command line tool to view objects (json/yaml/toml) in TUI tree widget

try the package

nix run github:anas-contribs/nixpkgs/by-name/otree#otree

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.11 Release Notes (or backporting 23.11 and 24.05 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.

@0x61nas 0x61nas force-pushed the by-name/otree branch 2 times, most recently from bff636b to ad6dd98 Compare June 1, 2024 18:07
afh

This comment was marked as outdated.

Sign in to view

@0x61nas

This comment was marked as outdated.

Sign in to view
@afh

This comment was marked as outdated.

Sign in to view
@0x61nas

This comment was marked as outdated.

Sign in to view
afh
afh approved these changes Jun 3, 2024
View reviewed changes
Copy link
Member

@afh afh left a comment

Choose a reason for hiding this comment

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

Great work on this, @0x61nas, and thank you for the contribution. I look forward to using otree in the future!

If you wanted to go above and beyond with this package run it file through nixmft…

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jun 4, 2024
afh

This comment was marked as resolved.

Sign in to view

@afh afh removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jun 6, 2024
afh
afh approved these changes Jun 6, 2024
View reviewed changes
Copy link
Member

@afh afh left a comment

Choose a reason for hiding this comment

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

Thanks for addressing this so quickly, @0x61nas, much appreciated!

@afh afh added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jun 6, 2024
@0x61nas 0x61nas added the 8.has: package (new) This PR adds a new package label Jun 9, 2024
@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-ready-for-review/3032/4071

@0x61nas 0x61nas added needs_merger (old Marvin label, do not use) needs_reviewer (old Marvin label, do not use) and removed needs_merger (old Marvin label, do not use) labels Jun 11, 2024
@0x61nas @afh
otree: init at 0.2.0
a4d30f6 
Co-authored-by: Alexis Hildebrandt <[email protected]>
@0x61nas 0x61nas changed the title otree: init at 0.1.0 otree: init at 0.2.0 Jun 29, 2024
@0x61nas 0x61nas removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jun 29, 2024
@0x61nas 0x61nas requested a review from afh June 29, 2024 07:59
afh
afh approved these changes Jul 1, 2024
View reviewed changes
Copy link
Member

@afh afh left a comment

Choose a reason for hiding this comment

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

How fortunate, that version 0.2.0 was released while this PR is still under review and leaveDotGit and git in nativeBuildInputs are no longer necessary 🥳

If you wanted to go above and beyond with this PR, please consider formatting it using:
nix run github:NixOS/nixfmt -- pkgs/by-name/ot/otree/package.nix

@afh
Copy link
Member

afh commented Jul 1, 2024

@0x61nas any idea what could cause the ofborg-eval check failure?

@0x61nas
Copy link
Member Author

0x61nas commented Jul 1, 2024

@0x61nas any idea what could cause the ofborg-eval check failure?

at the first, i thought that it was because of our (hacky) featching, but now i am not sure why

@0x61nas 0x61nas added 12.approvals: 1 This PR was reviewed and approved by one reputable person 11.by: package-maintainer This PR was created by the maintainer of the package it changes labels Jul 1, 2024
@IldenH
Copy link

IldenH commented Jul 14, 2024

Format using nixfmt-rfc-style
nix run nixpkgs#nixfmt-rfc-style -- pkgs/by-name/ot/otree/package.nix

@nbraud

This comment was marked as resolved.

Sign in to view
@nbraud
Copy link
Contributor

nbraud commented Sep 18, 2024

@0x61nas, sorry it took ages for someone to come identify the problem, and thanks for packaging otree!

I think that this should be good to go with the suggested fix... and a round of nixfmt if that wasn't already done: the CI now enforces the style guide for new files and those which were previously formatted.

@nbraud nbraud removed the needs_reviewer (old Marvin label, do not use) label Sep 18, 2024
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Oct 29, 2024
FliegendeWurst
FliegendeWurst requested changes Nov 11, 2024
View reviewed changes
pkgs/by-name/ot/otree/package.nix
@@ -0,0 +1,39 @@
{ lib
, fetchFromGitHub
Copy link
Member

Choose a reason for hiding this comment

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

Please format with the new formatter.

@FliegendeWurst FliegendeWurst added the awaiting_changes (old Marvin label, do not use) label Nov 11, 2024
@KiaraGrouwstra KiaraGrouwstra mentioned this pull request Nov 23, 2024
Merged
13 tasks
@Aleksanaa
Copy link
Member

Done in #358397

@Aleksanaa Aleksanaa closed this Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nbraud nbraud nbraud left review comments

@FliegendeWurst FliegendeWurst FliegendeWurst requested changes

@afh afh afh approved these changes

Assignees
No one assigned
Labels
8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes awaiting_changes (old Marvin label, do not use)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@0x61nas @afh @nixos-discourse @IldenH @nbraud @Aleksanaa @FliegendeWurst @wegank