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

rsClock: init at 0.1.0 #67467

Merged
merged 2 commits into from Aug 27, 2019
Merged

rsClock: init at 0.1.0 #67467

merged 2 commits into from Aug 27, 2019

Conversation

valebes
Copy link
Contributor

@valebes valebes commented Aug 25, 2019

Motivation for this change

A simple tty clock written in Rust.
https://github.com/valebes/rsClock

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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.
Notify maintainers

cc @

@mmahut
Copy link
Member

mmahut commented Aug 26, 2019

@GrahamcOfBorg build rsclock

Copy link
Member

@aanderse aanderse 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 choosing to contribute to nixpkgs! I've left a few comments I hope you'll find helpful.

Please squash/organize your commits such that you have a commit adding yourself to the maintainer list, followed by a commit adding the package.

pkgs/applications/misc/rsclock/default.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/rsclock/default.nix Outdated Show resolved Hide resolved
maintainers/maintainer-list.nix Outdated Show resolved Hide resolved
@valebes
Copy link
Contributor Author

valebes commented Aug 26, 2019

cc @aanderse
Thanks for the tips!

Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

Looking good! Just squash the last commit into the previous and I think you're good for merge.

@alexarice
Copy link
Contributor

I think you've squashed one too far here, I believe @aanderse wanted there to be 2 commits

@valebes valebes requested a review from FRidh as a code owner August 27, 2019 08:10
@valebes
Copy link
Contributor Author

valebes commented Aug 27, 2019

Holy cow, made a disaster.

@alexarice
Copy link
Contributor

could try git reflog

@alexarice
Copy link
Contributor

Definitely easier to revert to before when you made the rebase then redo it as the rebase has coauthored you in some commits

@valebes
Copy link
Contributor Author

valebes commented Aug 27, 2019

@alexarice
It was a nightmare :-) .
I hope now will be all right.

@valebes
Copy link
Contributor Author

valebes commented Aug 27, 2019

Hooray!
Ready for merge.

@mmahut
Copy link
Member

mmahut commented Aug 27, 2019

@GrahamcOfBorg build rsclock

Copy link
Contributor

@alexarice alexarice left a comment

Choose a reason for hiding this comment

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

Commits look good to me now

Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@mmahut
Copy link
Member

mmahut commented Aug 27, 2019

@GrahamcOfBorg build rsclock

@mmahut
Copy link
Member

mmahut commented Aug 27, 2019

@valebes thank you for your contribution and welcome to the community! :)

@mmahut mmahut merged commit 0245bfa into NixOS:master Aug 27, 2019
@valebes
Copy link
Contributor Author

valebes commented Aug 27, 2019

@mmahut thank you for the supports! :-)

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

5 participants