Skip to content

ui: remove monospace font for human-readable job description#35652

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
celiala:FixNonSQLFont
Mar 12, 2019
Merged

ui: remove monospace font for human-readable job description#35652
craig[bot] merged 1 commit intocockroachdb:masterfrom
celiala:FixNonSQLFont

Conversation

@celiala
Copy link
Collaborator

@celiala celiala commented Mar 12, 2019

In #35439, we introduced a user-friendly message (instead of the
exact SQL statement) for automatic table stats.

This commit uses the default font for that message, reserving
the monospace font for just SQL text.

cc @rolandcrosby @piyush-singh

image

@celiala celiala requested review from a team, RaduBerinde, rytaft and vilterp March 12, 2019 16:30
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @celiala, @RaduBerinde, @rytaft, and @vilterp)


pkg/ui/src/views/jobs/index.tsx, line 202 at r2 (raw file):

    title: "Description",
    cell: job => {
      if (job.type === jobTypeAutoCreateStats) {

I think the more general thing to check is if job.statement is not empty (when the field is empty, the description is the statement).

Copy link
Collaborator Author

@celiala celiala left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @RaduBerinde, @rytaft, and @vilterp)


pkg/ui/src/views/jobs/index.tsx, line 202 at r2 (raw file):

Previously, RaduBerinde wrote…

I think the more general thing to check is if job.statement is not empty (when the field is empty, the description is the statement).

ah, thanks - will do!

@celiala
Copy link
Collaborator Author

celiala commented Mar 12, 2019

TFTR!

bors r+

Copy link
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @rytaft and @vilterp)


pkg/ui/src/views/jobs/index.tsx, line 202 at r2 (raw file):

Previously, celiala wrote…

ah, thanks - will do!

[nit] probably a good idea to leave a comment explaining this

@celiala
Copy link
Collaborator Author

celiala commented Mar 12, 2019

bors r-

@craig
Copy link
Contributor

craig bot commented Mar 12, 2019

Canceled

@celiala
Copy link
Collaborator Author

celiala commented Mar 12, 2019

oh wait, I think that cancels everybody's -- will fix the nit in a future commit

bors r;-bors r+

In cockroachdb#35439, we introduced a user-friendly message (instead of the
exact SQL statement) for automatic table stats.

This commit uses the default font for that message, reserving
the monospace font for just SQL text.

Release note: None
@celiala
Copy link
Collaborator Author

celiala commented Mar 12, 2019

TFTRs!

  • added comment [and fixed a style regression I would have introduced].

bors r+

craig bot pushed a commit that referenced this pull request Mar 12, 2019
35607: roachtest: Renenable SQLSmith roachtest r=BramGruneir a=BramGruneir

Note that this is the original roachtest and not the re-written one.  If we find
that this one does not find any different issues, we should remove it entirely.
But there is no harm in enabling it right now.

Release note: None

35623: storage: enable follower_reads by default r=ajwerner a=ajwerner

This PR enables storage level follower reads by default. Follower reads still
will not be used for request routing without an enterprise license.

Release note: None

35645: sql/sqlbase: fix TestEncDatumSize under go1.12 r=knz a=petermattis

Fixes #35636.

golang/go@bfc54bb
reduced the memory usage of small big integers which in turn reduced the
`EncDatum.Size()` for the decimal datum used in this test.

Release note: None

35652: ui: remove monospace font for human-readable job description r=celiala a=celiala

In #35439, we introduced a user-friendly message (instead of the
exact SQL statement) for automatic table stats.

This commit uses the default font for that message, reserving
the monospace font for just SQL text.

cc @rolandcrosby @piyush-singh

![image](https://user-images.githubusercontent.com/3051672/54228134-e27cd500-44d7-11e9-8272-70da6ca7315c.png)


35654: roachtest: two easy changefeed test fixes r=nvanbenschoten a=danhhz

See commits for details

Co-authored-by: Bram Gruneir <bram@cockroachlabs.com>
Co-authored-by: Andrew Werner <ajwerner@cockroachlabs.com>
Co-authored-by: Peter Mattis <petermattis@gmail.com>
Co-authored-by: Celia La <celia@cockroachlabs.com>
Co-authored-by: Daniel Harrison <daniel.harrison@gmail.com>
@craig
Copy link
Contributor

craig bot commented Mar 12, 2019

Build succeeded

@craig craig bot merged commit e7eaca1 into cockroachdb:master Mar 12, 2019
@celiala celiala deleted the FixNonSQLFont branch March 15, 2019 19:21
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