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

semiphemeral: init at 0.6 #120619

Merged
merged 2 commits into from Apr 26, 2021
Merged

semiphemeral: init at 0.6 #120619

merged 2 commits into from Apr 26, 2021

Conversation

amanjeev
Copy link
Contributor

Motivation for this change

Adds python package Semiphemeral.

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

@r-rmcgibbo
Copy link

r-rmcgibbo commented Apr 25, 2021 •
edited

Result of nixpkgs-review pr 120619 at 34ebeb4f run on aarch64-linux 1

2 packages built successfully:
  • python38Packages.semiphemeral
  • python39Packages.semiphemeral
1 suggestion:
  • warning: no-python-tests

    Test runner could not discover any test cases: ‘Ran 0 tests in 0.000s’
    Near pkgs/development/python-modules/semiphemeral/default.nix:26:0:

       |
26 |     description = "Automatically delete your old tweets, except for the ones you want to keep";
   | ^

Result of nixpkgs-review pr 120619 at 34ebeb4f run on x86_64-linux 1

2 packages built successfully:
  • python38Packages.semiphemeral
  • python39Packages.semiphemeral
1 suggestion:
  • warning: no-python-tests

    Test runner could not discover any test cases: ‘Ran 0 tests in 0.000s’
    Near pkgs/development/python-modules/semiphemeral/default.nix:26:0:

       |
26 |     description = "Automatically delete your old tweets, except for the ones you want to keep";
   | ^

dotlambda
dotlambda requested changes Apr 25, 2021
View reviewed changes
Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

This isn't a Python module (no import semiphemeral), so put it in pkgs/tools/....

Also, you need exactly two commits:

  • maintainers: add amanjeev and
  • semiphemeral: init at 0.6

pkgs/development/python-modules/semiphemeral/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/semiphemeral/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/semiphemeral/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/semiphemeral/default.nix Outdated Show resolved Hide resolved
@dotlambda dotlambda changed the title python package - semiphemeral -> init at version 0.6 semiphemeral: init at version 0.6 Apr 25, 2021
@amanjeev amanjeev force-pushed the semiphemeral-python-package branch from 34ebeb4 to b806426 Compare April 25, 2021 20:20
dotlambda
dotlambda requested changes Apr 25, 2021
View reviewed changes
Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

Please also address @r-rmcgibbo's suggestion.

pkgs/tools/social/twitter/semiphemeral/default.nix Outdated Show resolved Hide resolved
pkgs/tools/social/twitter/semiphemeral/default.nix Outdated Show resolved Hide resolved
pkgs/tools/social/twitter/semiphemeral/default.nix Outdated Show resolved Hide resolved
@dotlambda
Copy link
Member

Mind the two commits.

@amanjeev
Copy link
Contributor Author

Would it be ok to squash the commits before merging or should I squash and push again?

dotlambda
dotlambda requested changes Apr 25, 2021
View reviewed changes
Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

You should set doCheck = false and add a comment since upstream seems to have no tests.

pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
@dotlambda
Copy link
Member

Would it be ok to squash the commits before merging or should I squash and push again?

I can only squash multiple commits into a single one when merging.

@amanjeev
Copy link
Contributor Author

Mind the two commits.

Yea I am can squash n -1 on my side and push again once done?

@dotlambda dotlambda changed the title semiphemeral: init at version 0.6 semiphemeral: init at 0.6 Apr 25, 2021
@amanjeev amanjeev force-pushed the semiphemeral-python-package branch from 72149e0 to 46fef30 Compare April 25, 2021 22:54
@amanjeev amanjeev requested a review from dotlambda April 25, 2021 23:12
dotlambda
dotlambda approved these changes Apr 26, 2021
View reviewed changes
Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

LGTM except for the path.

@amanjeev amanjeev force-pushed the semiphemeral-python-package branch from 46fef30 to 4e9791e Compare April 26, 2021 13:50
@amanjeev
Copy link
Contributor Author

thank you so much @dotlambda for your patience with me

Hoverbear
Hoverbear approved these changes Apr 26, 2021
View reviewed changes
Copy link
Member

@Hoverbear Hoverbear left a comment

Choose a reason for hiding this comment

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

Tested on NixOS unstable and works fine.

@amanjeev
Copy link
Contributor Author

@Hoverbear suggested I use nixpjgs-fmt before committing. Will do and push! Sorry about this.

@amanjeev
semiphemeral: init at 0.6
281efa2 
semiphemeral: init at 0.6

semiphemeral: init at 0.6

semiphemeral: init at 0.6

semiphemeral: fix formatting

semiphemeral: refactor for flexibility

semiphemeral: remove extraneous inherit

semiphemeral: no check since upstream has no tests

semiphemeral: refactor move to misc instead of social/twitter

semiphemeral: fix formatting
@amanjeev amanjeev force-pushed the semiphemeral-python-package branch from 4e9791e to 281efa2 Compare April 26, 2021 15:11
@dotlambda dotlambda merged commit 53d547c into NixOS:master Apr 26, 2021
@amanjeev amanjeev deleted the semiphemeral-python-package branch April 26, 2021 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotlambda dotlambda dotlambda requested changes

@Hoverbear Hoverbear Hoverbear approved these changes

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@amanjeev @r-rmcgibbo @dotlambda @Hoverbear