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

stale bot: even more TL;DR #101320

Merged
merged 1 commit into from Oct 25, 2020
Merged

stale bot: even more TL;DR #101320

merged 1 commit into from Oct 25, 2020

Conversation

@ryantm
Copy link
Member

@ryantm ryantm commented Oct 22, 2020

Continuation of #100462 since I couldn't figure out how to make a PR against @blaggacao's repo.

The goal here is to make the stale bot's message super short, and direct people to a documentation page that we can update over time.

@blaggacao
Copy link
Contributor

@blaggacao blaggacao commented Oct 22, 2020

Excellent! Thanks also for the rewordings!

.github/STALE-BOT.md Outdated Show resolved Hide resolved
Copy link
Contributor

@blaggacao blaggacao left a comment

Formatting Oversight

Copy link
Contributor

@doronbehar doronbehar left a comment

Looks Great to me. Good thinking!

@ryantm ryantm force-pushed the ryantm:stale branch 2 times, most recently from bb06507 to 1e1c715 Oct 22, 2020
@ryantm
Copy link
Member Author

@ryantm ryantm commented Oct 22, 2020

I resolved my formatting mistake and added a Thank you to the top.

cc @abathur who cares about this too.

@abathur
Copy link
Member

@abathur abathur commented Oct 22, 2020

I either didn't stay in the loop or have simply forgotten, but I didn't realize the current messages were this big. It's good to give information, but I agree the message is trying to do much work.

What sort of feedback/edits are you looking for? I don't have time to read everything or copyedit the manual section just this moment, but I think I'd like to at least make a suggested edit that rounds the corners of the new message later today.

@ryantm
Copy link
Member Author

@ryantm ryantm commented Oct 23, 2020

@abathur I'm planning to merge this tomorrow, so if your suggestions don't happen by then, feel free to do a separate PR!

@ryantm
Copy link
Member Author

@ryantm ryantm commented Oct 23, 2020

@abathur Sorry, I didn't answer your question. I was looking for proof reading/copy editing, since the text changed a lot. The most important part to get right is the short markComment part though, since we can dynamically change the other part.

@abathur
Copy link
Member

@abathur abathur commented Oct 23, 2020

I'll try to double-dip some build time to look at it this evening :)

Edit: I didn't explain most of these, but feel free to haggle or ask for reasoning.

.github/STALE-BOT.md Outdated Show resolved Hide resolved
.github/STALE-BOT.md Outdated Show resolved Hide resolved
.github/STALE-BOT.md Outdated Show resolved Hide resolved
.github/STALE-BOT.md Outdated Show resolved Hide resolved
.github/STALE-BOT.md Outdated Show resolved Hide resolved
.github/stale.yml Outdated Show resolved Hide resolved
.github/STALE-BOT.md Outdated Show resolved Hide resolved
.github/STALE-BOT.md Outdated Show resolved Hide resolved
.github/STALE-BOT.md Outdated Show resolved Hide resolved
.github/STALE-BOT.md Show resolved Hide resolved
@ryantm ryantm force-pushed the ryantm:stale branch from a80b30d to 9d6dde7 Oct 24, 2020
@ryantm
Copy link
Member Author

@ryantm ryantm commented Oct 24, 2020

squashed all those commits into one

@blaggacao
Copy link
Contributor

@blaggacao blaggacao commented Oct 24, 2020

@abathur Thanks for your rewordings. Spot on.

@ryantm I really like the conciseness of just linking to the help. It feels like being consequent about TL;DR and people will respect that (and find respecfulness within it).

moves all the information to the Markdown document that we can
dynamically update and tries to make things more concise and scannable.

Co-authored-by: Ryan Mulligan <ryan@ryantm.com>
Co-authored-by: Travis A. Everett <travis.a.everett@gmail.com>
@ryantm ryantm force-pushed the ryantm:stale branch from 9d6dde7 to 27510ad Oct 25, 2020
@Mic92
Mic92 approved these changes Oct 25, 2020
Copy link
Member

@Mic92 Mic92 left a comment

Good to merge!

@ryantm ryantm merged commit 6e61e0d into NixOS:master Oct 25, 2020
16 checks passed
16 checks passed
@github-actions
tests
Details
@github-actions
action
Details
@ofborg
Evaluation Performance Report Evaluator Performance Report
Details
@github-actions
Wait for ofborg
Details
@ofborg
grahamcofborg-eval ^.^!
Details
@ofborg
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
@ofborg
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="27510ad"; rev="27510ad13fa4558b1744f41aa68cae18ccfdf630"; } ./pkgs/t
Details
@ofborg
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
@ofborg
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="27510ad"; rev="27510ad13fa4558b1744f41aa68cae18ccfdf630"; } ./nixos/
Details
@ofborg
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="27510ad"; rev="27510ad13fa4558b1744f41aa68cae18ccfdf630"; } ./nixos/
Details
@ofborg
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="27510ad"; rev="27510ad13fa4558b1744f41aa68cae18ccfdf630"; } ./nixos/
Details
@ofborg
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="27510ad"; rev="27510ad13fa4558b1744f41aa68cae18ccfdf630"; } ./pkgs/t
Details
@ofborg
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="27510ad"; rev="27510ad13fa4558b1744f41aa68cae18ccfdf630"; } ./pkgs/t
Details
@ofborg
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="27510ad"; rev="27510ad13fa4558b1744f41aa68cae18ccfdf630"; } ./pkgs/t
Details
@ofborg
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
@ofborg
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@cole-h
Copy link
Member

@cole-h cole-h commented Oct 26, 2020

I think this needs at least some form of short introduction. Just linking to another document without any other information seems... bad, to me. See #85956 (comment) -- just dropping a link doesn't feel like a good user experience to me, at least without some kind of short explanation.

@ryantm
Copy link
Member Author

@ryantm ryantm commented Oct 26, 2020

@cole-h Thanks for the feedback! Here's our reasons for making it super short:

  1. We can't change all the old comments it makes if something changes
  2. We'd like people to read the more detailed page
  3. We don't want to annoy regular contributors with a long message

How does that sit with you? Do you have a suggestion or what to change?

@blaggacao
Copy link
Contributor

@blaggacao blaggacao commented Oct 26, 2020

Maye a compromise could be:

I marked this as stale. → More Info

@cole-h
Copy link
Member

@cole-h cole-h commented Oct 26, 2020

@ryantm I definitely agree with all of your reasoning, but I also think there ought to be a small "springboard" (if you will), as @blaggacao suggested.

I would lean more towards a message like the following:

I've marked this issue as stale due to inactivity. For more information, click here.

or something similar. It gives a "why", and then links to the document, instead of just dropping a link and saying "cya".

@blaggacao
Copy link
Contributor

@blaggacao blaggacao commented Oct 26, 2020

@cole-h Sounds good, just avoid "issue" or "pr" to make the text ubiquitous.

I marked this as stale due to inactivity. → More Info

@blaggacao
Copy link
Contributor

@blaggacao blaggacao commented Oct 26, 2020

#101781

@colemickens
Copy link
Member

@colemickens colemickens commented Nov 16, 2020

Am I missing something or does this ... not actually tell the user how to remove the stale tag from an issue that is still important?

@ryantm
Copy link
Member Author

@ryantm ryantm commented Nov 16, 2020

@colemickens the doc it links to says "To remove the stale label, just leave a new comment" very prominently.

@colemickens
Copy link
Member

@colemickens colemickens commented Nov 16, 2020

I don't know why I glazed over that and went straight to "suggestions for PRs" (and then skimmed the text for any monospaced font with the key phrase), but indeed, it does clearly say to just leave a comment. My mistake.

@ryantm ryantm deleted the ryantm:stale branch Nov 26, 2020
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

7 participants