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

Replace short URL from #63103 with full URL. #77768

Merged
merged 1 commit into from Jan 15, 2020
Merged

Conversation

@dhess
Copy link
Contributor

dhess commented Jan 15, 2020

It's polite to show a user where a URL is taking them before they follow it. Also, the message is already (helpfully) verbose, so there's not much benefit in using a shortener in the first place.

This change also makes the documentation consistent with existing practice when referring to GitHub pull requests in user-visible strings; see, for example:

https://github.com/NixOS/nixpkgs/blob/cd6ac55fb1b4dc75418550cdd75edc8522bf1e9b/pkgs/top-level/all-packages.nix#L25036

"See: https://github.com/NixOS/nixpkgs/pull/57681#issuecomment-475857499"

"Numerous and uncounted, upstream has requested we not package it. See more: https://github.com/NixOS/nixpkgs/pull/23058#issuecomment-283515104"

The error message is already helpfully verbose, so there's little
reason to shorten the informational URL.
@dhess dhess requested review from edolstra, Infinisil and nbp as code owners Jan 15, 2020
@@ -406,7 +406,7 @@ rec {
In file ${def.file}
a list is being assigned to the option config.${option}.
This will soon be an error as type loaOf is deprecated.
See https://git.io/fj2zm for more information.
See https://github.com/NixOS/nixpkgs/pull/63103 for more information.

This comment has been minimized.

Copy link
@grahamc

grahamc Jan 15, 2020

Member

Should we wrap this at more?

This comment has been minimized.

Copy link
@dhess

dhess Jan 15, 2020

Author Contributor

I thought about it, but decided instead to go with the idea that the original author intended there to be one full sentence per line. (The templated bits in the displayed message often greatly exceed the line length of the line I added.)

@grahamc grahamc merged commit 63dd084 into NixOS:master Jan 15, 2020
12 checks passed
12 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.