python311Packages.shimmy: init at 1.3.0, python311Packages.dm-control: init at 1.0.19, python311Packages.noxe-xunitmp: init at 0.4.1 #309980
Conversation
| src = fetchFromGitHub { | ||
| owner = "google-deepmind"; | ||
| repo = "dm_control"; | ||
| rev = "refs/tags/${version}"; |
There was a problem hiding this comment.
| rev = "refs/tags/${version}"; | |
| rev = version; |
Lines 722 to 736 in 65d1532
There was a problem hiding this comment.
I completely agree that I should write version instead of "${version}", but in this case it is quite not the same.
A committer told me to do that when I started contributing. It was supposed to prevent branch/tag mismatches.
Do you have more up-to-date infos on this ?
There was a problem hiding this comment.
Waiting for @DontEatOreo before proceeding further. I have no opinion on this. One or the other it's fine for me.
There was a problem hiding this comment.
I think @GaetanLepage raises a valid concern, and actually, I agree with that committer. In that case, you can ignore this change. Also, could you please link to that committer's comment about this?
On another note, maybe there should be a PR that clarifies this in the manual or somewhere else? I'm currently looking at the manual and there are examples of rev = version; in many places. Searching for "rev = version;" on nixpkgs reveals it's widely used as well
There was a problem hiding this comment.
Fair enough, on my side I'll merge this PR once CI is done then.
There was a problem hiding this comment.
I think @GaetanLepage raises a valid concern, and actually, I agree with that committer. In that case, you can ignore this change. Also, could you please link to that committer's comment about this?
If I remember well, this was @SuperSandro2000, but I wouldn't be able to find the comment as it was ~1.5 years ago.
On another note, maybe there should be a PR that clarifies this in the manual or somewhere else? I'm currently looking at the manual and there are examples of
rev = version;in many places. Searching for"rev = version;"on nixpkgs reveals it's widely used as well
I asked around and no-one seems to have a strong preference.
Relevant work: #153386
ofborg
bot
added
8.has: package (new)
|
Result of 6 packages built:
|
|
Result of 6 packages built:
|
|
Could you update the PR title to include all three packages so it's clearer what is being added |
GaetanLepage
changed the title
|
Result of 6 packages built:
|
Description of changes
Add python package shimmy, an API conversion tool for popular external reinforcement learning environments.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.