Skip to content

monophony: init at 2.3.0#263242

Merged
natsukium merged 2 commits intoNixOS:masterfrom
Henry-Hiles:init-monophony
Oct 31, 2023
Merged

monophony: init at 2.3.0#263242
natsukium merged 2 commits intoNixOS:masterfrom
Henry-Hiles:init-monophony

Conversation

@Henry-Hiles
Copy link
Contributor

@Henry-Hiles Henry-Hiles commented Oct 24, 2023 •
edited
Loading

Description of changes

Adds monophony: https://gitlab.com/zehkira/monophony
Fixes #217848

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/)
  • 23.11 Release Notes (or backporting 23.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.

@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Oct 24, 2023
@Henry-Hiles
Copy link
Contributor Author

Henry-Hiles commented Oct 24, 2023

It's failing because of tabs, not spaces, but i ran nixpkgs-fmt on it, so I'm not sure why.

@eclairevoyant
Copy link
Contributor

eclairevoyant commented Oct 24, 2023

Don't use tabs, always 2 space indentation, as documented in contributing.md. nixpkgs-fmt is not the official formatter, so don't trust it.

@Henry-Hiles
Copy link
Contributor Author

Henry-Hiles commented Oct 24, 2023

Oh, what formatter is official?

eclairevoyant
eclairevoyant suggested changes Oct 24, 2023
View reviewed changes
eclairevoyant
eclairevoyant suggested changes Oct 24, 2023
View reviewed changes
@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 Oct 24, 2023
@Henry-Hiles
Copy link
Contributor Author

Henry-Hiles commented Oct 25, 2023 •
edited
Loading

I fixed some of these issues (namely longDescription and mainProgram)

@Henry-Hiles
Copy link
Contributor Author

Henry-Hiles commented Oct 25, 2023 •
edited
Loading

Are all of these issues now fixed?

ghost
ghost reviewed Oct 25, 2023
View reviewed changes
Copy link

@ghost ghost left a comment •
edited by ghost
Loading

Choose a reason for hiding this comment

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

looks good. a few comments. i think using buildPythonPath is preferred over $PYTHONPATH as it doesn't include some unnecessary dependencies. it also builds a PATH that contains yt-dlp.

maybe name the mpris-server package mpris_server so it matches the python import?

ghost
ghost reviewed Oct 25, 2023
View reviewed changes
@Henry-Hiles Henry-Hiles changed the title init: monophony at 4.2.2 init: monophony at 2.3.0 Oct 25, 2023
ghost
ghost reviewed Oct 25, 2023
View reviewed changes
ghost
ghost approved these changes Oct 25, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM!
build and ran on my machine x64 linux.

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

2 packages built:
  • python310Packages.mpris_server
  • python310Packages.mpris_server.dist

@Henry-Hiles Henry-Hiles added the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 25, 2023
ghost
ghost reviewed Oct 25, 2023
View reviewed changes
ghost
ghost reviewed Oct 25, 2023
View reviewed changes
ghost
ghost reviewed Oct 26, 2023
View reviewed changes
ghost
ghost approved these changes Oct 27, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

looks good. i merged in master, built and ran on x64 linux.

ghost
ghost approved these changes Oct 29, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM! built and ran on x64 linux. verified python-3.11 via nix-store -q --references

@Henry-Hiles Henry-Hiles added the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 29, 2023
@delroth delroth removed the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 29, 2023
@Henry-Hiles
Copy link
Contributor Author

Henry-Hiles commented Oct 29, 2023 •
edited
Loading

@delroth why did you remove the approval? All I did was remove a single unused import.

@h7x4
Copy link
Member

h7x4 commented Oct 30, 2023

@Henry-Hiles no need to worry about it. I believe it's a script that gets run every so often, which checks how many people has approved a PR. If any changes has happened in between the approval and the current, the script will not consider it approved in the current state, no matter how insignificant the change is.

@Henry-Hiles
Copy link
Contributor Author

Henry-Hiles commented Oct 30, 2023

I see, thanks!

@Henry-Hiles Henry-Hiles requested a review from natsukium October 30, 2023 01:56
@ghost ghost self-requested a review October 30, 2023 02:07
@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 30, 2023
@Henry-Hiles
Copy link
Contributor Author

Henry-Hiles commented Oct 30, 2023

Hmm, im suddenly getting Playback error: No URI handler implemented for "https". again, i cant play songs

@Henry-Hiles
Copy link
Contributor Author

Henry-Hiles commented Oct 30, 2023

Would you please change the commit message to python311Packages.mpris-server: init at 0.4.2?

Oh i forgot about this too, ill do this soon.

@Henry-Hiles
Copy link
Contributor Author

Henry-Hiles commented Oct 30, 2023

Hmm, im suddenly getting Playback error: No URI handler implemented for "https". again, i cant play songs

I can't figure out where this was introduced, any ideas?

@Henry-Hiles
Copy link
Contributor Author

Henry-Hiles commented Oct 30, 2023

LGTM! built and ran on x64 linux. verified python-3.11 via nix-store -q --references

I assume songs played fine here, correct?

@ghost
Copy link

ghost commented Oct 30, 2023

LGTM! built and ran on x64 linux. verified python-3.11 via nix-store -q --references

I assume songs played fine here, correct?

it is playing songs for me.
try removing ~/.cache/gstreamer-1.0 and trying again.

@Henry-Hiles
Copy link
Contributor Author

Henry-Hiles commented Oct 30, 2023

Sure

@Henry-Hiles
Copy link
Contributor Author

Henry-Hiles commented Oct 30, 2023 •
edited
Loading

That worked, thanks!

@Henry-Hiles
Copy link
Contributor Author

Henry-Hiles commented Oct 30, 2023

Would you please change the commit message to python311Packages.mpris-server: init at 0.4.2?

Done

@Henry-Hiles Henry-Hiles requested review from a user, Tungsten842 and eclairevoyant October 30, 2023 18:12
@delroth delroth removed the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 30, 2023
@Henry-Hiles Henry-Hiles added the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 30, 2023
natsukium
natsukium approved these changes Oct 31, 2023
View reviewed changes
Copy link
Member

@natsukium natsukium left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@natsukium natsukium merged commit 3959b83 into NixOS:master Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@natsukium natsukium natsukium approved these changes

@Tungsten842 Tungsten842 Awaiting requested review from Tungsten842

@eclairevoyant eclairevoyant Awaiting requested review from eclairevoyant

+1 more reviewer
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: python Python is a high-level, general-purpose programming language. 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-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Monophony

6 participants

@Henry-Hiles @eclairevoyant @natsukium @h7x4 @Tungsten842 @delroth