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

repology: fix homepage for a few packages - nothing else #59265

Merged
merged 4 commits into from Apr 11, 2019

Conversation

peterhoeg
Copy link
Member

Motivation for this change

repology was showing a few errors for packages I maintain - think of it as spring cleaning.

No other changes.

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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@peterhoeg peterhoeg requested a review from FRidh as a code owner April 10, 2019 13:34
@peterhoeg
Copy link
Member Author

@GrahamcOfBorg build puddletag krename anydesk broadlink-cli

@peterhoeg peterhoeg self-assigned this Apr 10, 2019
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.

"broadlink" -> "pythonPackages.broadlink"

nativeBuildInputs = [ extra-cmake-modules kdoctools wrapGAppsHook ];

propagatedBuildInputs = [ kconfig kcrash kinit kjsembed ];

Copy link
Contributor

Choose a reason for hiding this comment

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

No other changes, eh?

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, "cleanup" might be a better commit message

Copy link
Contributor

Choose a reason for hiding this comment

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

Not even that, there are new dependencies imported in krename. I would split that out and explain why in the commit message.

For the "update homepage" parts of the patchset, I'd approve... but merging stuff with the wrong commit message is a no-no for me.

😄

Copy link
Member

@dotlambda dotlambda Apr 10, 2019

Choose a reason for hiding this comment

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

Not even that, there are new dependencies imported in krename. I would split that out and explain why in the commit message.

There aren't. The lines are just moved.
That's why @GrahamcOfBorg added the "10.rebuild-linux: 0" label.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, that's actually true. Sorry about that!

Still, the commit message should be more explanatory about that ("Moved some attributed for better readability" would be fine for me, I guess).

Copy link
Member Author

Choose a reason for hiding this comment

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

Moving a few lines up before the meta tag doesn't change anything in terms of input (and hence output). But fair enough, I have amended the commit to mention the minor style changes.

Copy link
Contributor

@matthiasbeyer matthiasbeyer left a comment

Choose a reason for hiding this comment

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

These are not only homepage link fixes.

@peterhoeg
Copy link
Member Author

@dotlambda, good catch on the name - fixed.

@dotlambda dotlambda merged commit 84029e2 into NixOS:master Apr 11, 2019
@peterhoeg peterhoeg deleted the f/repology branch April 11, 2019 16:03
@peterhoeg peterhoeg restored the f/repology branch April 11, 2019 16:52
@peterhoeg peterhoeg deleted the f/repology branch April 25, 2019 02:48
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

4 participants