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

Jump to bottom

gmrender-resurrect: cc96ede -> 0.0.8 #71474

Merged
merged 5 commits into from Nov 16, 2019
Merged

gmrender-resurrect: cc96ede -> 0.0.8 #71474

merged 5 commits into from Nov 16, 2019

Conversation

ashkitten
Copy link
Contributor

Motivation for this change

was marked broken, now updated with hzeller/gmrender-resurrect#190 and tested to work. also, added myself as a maintainer because i plan to keep this up to date in the future as i use it

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 @k0ral

@ashkitten ashkitten changed the title Update gmrender resurrect gmrender-resurrect: cc96ede -> v0.0.8 Oct 21, 2019
@ashkitten
Copy link
Contributor Author

the first release of gmrender-resurrect was just published, so i changed the pull request to reflect that. theoretically, we should be able to follow releases now!

c0bw3b
c0bw3b requested changes Nov 16, 2019
View reviewed changes
Copy link
Contributor

@c0bw3b c0bw3b left a comment •
edited

Choose a reason for hiding this comment

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

You can replace autoconf+automake with autoreconfHook in nativeBuildInputs too.

Then the explicit definitions of preConfigurePhases and autoconfPhase aren't needed anymore.

pkgs/tools/networking/gmrender-resurrect/default.nix Outdated Show resolved Hide resolved
@ashkitten
Copy link
Contributor Author

ashkitten commented Nov 16, 2019 •
edited

just incorporated suggestions and refactored the plugin path out to a function which generates it from a list of packages

c0bw3b
c0bw3b reviewed Nov 16, 2019
View reviewed changes
Copy link
Contributor

@c0bw3b c0bw3b left a comment •
edited

Choose a reason for hiding this comment

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

Woah the diff is weird now! Maybe you used another editor and it turned spaces into tabs.
+ inconsistent indentation in your let .. in block
+ why scope fetchpatch? doesn't seem to be used

@ashkitten
Copy link
Contributor Author

the let..in block was just one line before, so by adding more things i had to expand it out, thus the massive indentation change that made the diff effectively useless. sorry about that. i don't see any inconsistent indentation, though.

it seems i forgot to remove fetchpatch from before 0.0.8 was released and i was pulling in a patch to get it to compile. i'll remove that.

@c0bw3b
Copy link
Contributor

c0bw3b commented Nov 16, 2019

the let..in block was just one line before, so by adding more things i had to expand it out, thus the massive indentation change that made the diff effectively useless. sorry about that. i don't see any inconsistent indentation, though.

Oh ok! No you did nothing wrong then.
GH UI misled my eyes with those bold color blocks.. >;<

@ashkitten
Copy link
Contributor Author

apparently there is an option to hide whitespace changes behind the cog icon in the diff ui
image

@c0bw3b c0bw3b changed the title gmrender-resurrect: cc96ede -> v0.0.8 gmrender-resurrect: cc96ede -> 0.0.8 Nov 16, 2019
c0bw3b
c0bw3b approved these changes Nov 16, 2019
View reviewed changes
Copy link
Contributor

@c0bw3b c0bw3b left a comment

Choose a reason for hiding this comment

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

Builds and runs

@c0bw3b c0bw3b merged commit 19b9dd6 into NixOS:master Nov 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@c0bw3b c0bw3b c0bw3b approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ashkitten @c0bw3b