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

log: minor copyedits to fatal error postamble #30918

Merged
merged 1 commit into from
Oct 3, 2018

Conversation

couchand
Copy link
Contributor

@couchand couchand commented Oct 3, 2018

A followup to #30898 with a few minor edits.

@couchand couchand requested a review from tbg October 3, 2018 13:39
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@tbg tbg left a comment

Choose a reason for hiding this comment

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

:lgtm: mod comment

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale)


pkg/util/log/clog.go, line 63 at r1 (raw file):

Please submit a crash report by following the instructions here:

    https://github.com/cockroachdb/cockroach/issues/new?template=bug_report.md

I intentionally didn't hard-code the template here. We're releasing this, and we might change the template, which drops users into a void instead: https://github.com/cockroachdb/cockroach/issues/new?template=bug_report_nope.md

@couchand
Copy link
Contributor Author

couchand commented Oct 3, 2018

I intentionally didn't hard-code the template here. We're releasing this, and we might change the template

I had a similar thought. What if we added a lint that the template must exist?

OTOH, we could make a shortlink that redirects here and then we're insulated if it needs to change to anything else, too...

Copy link
Member

@tbg tbg left a comment

Choose a reason for hiding this comment

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

I personally would KISS here (the funnel will break as people fudge with the log files and realize they don't have a black hole to through them in, etc) but a lint sounds OK too. The shortlink option adds another link that can break so I'd rather not follow that direction.

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@couchand couchand force-pushed the edit/fatal-error-postamble branch from 8f5fd84 to ceacfc1 Compare October 3, 2018 14:00
@couchand
Copy link
Contributor Author

couchand commented Oct 3, 2018

Exactly right on the funnel: trying to remove a click was to shorten it.

Going with your original link for now to keep it simple. The choose page also feels more like a landing page, whereas the bug report template itself is another big wall of text to parse, good to have a moment in between them.

@couchand
Copy link
Contributor Author

couchand commented Oct 3, 2018

bors r=tschottdorf

craig bot pushed a commit that referenced this pull request Oct 3, 2018
30918: log: minor copyedits to fatal error postamble r=tschottdorf a=couchand

A followup to #30898 with a few minor edits.

Co-authored-by: Andrew Couch <[email protected]>
@tbg
Copy link
Member

tbg commented Oct 3, 2018

Thanks @couchand. I'll backport my change and slap this on top of it.

@craig
Copy link
Contributor

craig bot commented Oct 3, 2018

Build succeeded

@craig craig bot merged commit ceacfc1 into cockroachdb:master Oct 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants