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

cotp: init at 1.1.0 #200434

Merged
merged 2 commits into from
Jan 15, 2023
Merged

cotp: init at 1.1.0 #200434

merged 2 commits into from
Jan 15, 2023

Conversation

DavSanchez
Copy link
Contributor

This is a follow-up on #200427 which was closed due to me messing up while rebasing (sorry!)

In case I need additional changes, I'm opening it as a draft so I can take more time to review.

Description of changes
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/)
  • 22.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

pkgs/applications/misc/cotp/default.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/cotp/default.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/cotp/default.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/cotp/default.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/cotp/default.nix Outdated Show resolved Hide resolved
@DavSanchez
Copy link
Contributor Author

DavSanchez commented Nov 11, 2022

Sorry about the format issues @azahi , I read somewhere that alejandra was being used for formatting and applied it, but perhaps I read it somewhere else and mixed up. For future contributions, perhaps should I use nixfmt instead?

@azahi
Copy link
Member

azahi commented Nov 11, 2022

should I use nixfmt instead

No, don't use that. There are no specific guidelines on how to format Nix code for Nixpkgs (well, at least that I know of), just try to mimic what's already there. If you need a formatter you can use nixpkgs-fmt, but it's not "official" per se.

There's also an RFC about that if you are interested.

@azahi
Copy link
Member

azahi commented Nov 11, 2022

Evaluation is currently failing, you need to fix that and then please squash your commits. You should have two commits ready to merge: one which adds yourself as a maintainer, and one which creates the package itself. You can find out how to correctly format commit messages in CONTRIBUTING.md or just look at the git logs and see what other people are doing.

@azahi
Copy link
Member

azahi commented Nov 12, 2022

Looks great! The only think left is that you should remove the v prefix for version in your commit message, it should be like this: cotp: init at 1.1.0.

@DavSanchez DavSanchez changed the title cotp: init at v1.1.0 cotp: init at 1.1.0 Nov 12, 2022
@DavSanchez
Copy link
Contributor Author

Looks great! The only think left is that you should remove the v prefix for version in your commit message, it should be like this: cotp: init at 1.1.0.

Done!

Copy link
Member

@azahi azahi left a comment

Choose a reason for hiding this comment

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

Apparently I've missed these two:

pkgs/applications/misc/cotp/default.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/cotp/default.nix Outdated Show resolved Hide resolved
@azahi
Copy link
Member

azahi commented Nov 13, 2022

Result of nixpkgs-review pr 200434 run on x86_64-linux 1

1 package built:
  • cotp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants