First round of movr updates #5075
Conversation
|
This change is |
|
Online preview: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/80c7b619ff1e4aa881dd7a0561a193578bb368c1/ Edited pages: |
|
Can you please squash and force push at your leisure? Will make review easier for me. |
|
Online preview: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/f273e4626703294d170013a078d8718d46435349/ Edited pages: |
ericharmeling
force-pushed
the
movr-update
branch
from
f273e46 to
128f805
Compare
|
Online preview: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/128f805d454ae8f82bf721c2957eda37317a2315/ Edited pages: |
ericharmeling
force-pushed
the
movr-update
branch
from
9264dc3 to
0c7a13b
Compare
|
Online preview: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/9264dc38c97b62e86629ed23fcb94e6396c9c8da/ Edited pages: |
|
Online preview: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/0c7a13bb962238743052c2a4e1a16b5dbb555a61/ Edited pages: |
ericharmeling
force-pushed
the
movr-update
branch
from
3105477 to
17a452b
Compare
There was a problem hiding this comment.
Eric, this is an awesome first PR! You've done a ton of valuable work. Thanks for getting us closer to a consistently-exampled, automatically-testable future!
I have lots of comments, but tl;dr: it mostly LGTMs with one exception: I'm not convinced about the "break out 'Learn CockroachDB SQL' into multiple pages" approach. My strong preference is that we just update the existing "Learn CockroachDB SQL" page with MovR examples and leave it at that. I'm sorry I didn't respond to your ping on #5040, I definitely would have communicated with you about this earlier. For some reason I didn't get an email about that mention.
Either way, this is great - and I'm sure Jesse will also have opinions when he's back next week.
ericharmeling
force-pushed
the
movr-update
branch
from
17a452b to
0cf4fe2
Compare
There was a problem hiding this comment.
Wow! This is an excellent start to this project, and a huge effort! Thank you, @ericharmeling.
Please see my comments and questions.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @ericharmeling, @jseldess, and @rmloveland)
_includes/sidebar-data-v19.2.json, line 126 at r2 (raw file):
}, { "title": "Examples",
I don't think this is the best place for this doc. Instead, I'd recommend we create a new section under Reference called Sample Applications or something similar, and put it there. Thoughts?
_includes/v19.2/sql/movr-statements.md, line 5 at r1 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
SGTM
Happy to help think through conditional includes, Eric, if you'll explain more what end result you're looking for.
v19.2/cockroach-workload.md, line 90 at r2 (raw file):
-----|------------ `--drop` | Drop the existing database, if it exists, before loading the dataset. `--pprofport` | The port for pprof endpoint.<br><br>**Default:** `33333`
What is this exactly? I'm not getting much from the current description.
v19.2/create-index.md, line 54 at r2 (raw file):
## Examples {% include {{page.version.version}}/sql/movr-statements.md %}
This is great! What about giving this its own h3 heading like Setup just to make it more explicit at a TOC glance?
v19.2/create-table.md, line 98 at r2 (raw file):
## Examples {% include {{page.version.version}}/sql/movr-statements-no-data.md %}
This may be an example of a page where we don't get much use of movr since these are all create statements. Also, if users do set up movr ahead of time, they'll end up with errors due to the names of the tables in the default dataset. Please consider removing here.
v19.2/create-table.md, line 148 at r2 (raw file):
### Create a table (primary key defined) In this example, we create the `users` table, but with some column [constraints](constraints.html). One column is the [primary key](primary-key.html), and another column is given a [unique constraint](unique.html) and a [check constraint](check.html) that limits the length of the string. Primary key columns and columns with unique constraints are automatically indexed.
I think the schema should be identical to the one above but with the primary key defined. The new schema adds unrelated complexity by including the check constraint. If you want to highlight check in an example, perhaps add a separate section, Create a table with a check constraint, after Create a table with a foreign key constraint.
v19.2/delete.md, line 86 at r2 (raw file):
## Examples {% include {{page.version.version}}/sql/movr-statements.md %}
Same comment as earlier: Might want to make this an h3 section, Setup.
v19.2/delete.md, line 137 at r2 (raw file):
{% include copy-clipboard.html %} ~~~ sql > DELETE FROM promo_codes WHERE code = 'about_stuff_city' RETURNING *;
If you followed the first example, you're not going to get any results here. Would it be possible to make these examples operate on different segments of data? Maybe the first example should delete from a different table, for example.
v19.2/delete.md, line 137 at r2 (raw file):
{% include copy-clipboard.html %} ~~~ sql > DELETE FROM promo_codes WHERE code = 'about_stuff_city' RETURNING *;
Hmm, this pre-dates your changes, but I wonder if we should remove the RETURNING clause from this and the next query since returning is the focus of the next section?
v19.2/drop-table.md, line 34 at r2 (raw file):
## Examples {% include {{page.version.version}}/sql/movr-statements.md %}
Same comment as earlier: Might want to make this an h3 section, Setup.
v19.2/insert.md, line 584 at r2 (raw file):
~~~ You can also use a `WHERE` clause to apply the `DO UPDATE SET` expression conditionally:
No rows match this condition at this point, so you might want to rethink this a bit.
v19.2/learn-cockroachdb-sql.md, line 10 at r2 (raw file):
This page walks you through some of the most essential CockroachDB SQL statements. For a complete list and related details, see [SQL Statements](sql-statements.html). {% unless site.managed %}
This logic was in place to create slightly different outputs for regular CockroachDB docs vs the same page in the Managed CockroachDB docs. As is, the new version does not work for Managed CockroachDB. Compare http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/0cf4fe269ac9d714ad93eb0d5e01eeed96809384/managed/dev/learn-cockroachdb-sql.html to http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/0cf4fe269ac9d714ad93eb0d5e01eeed96809384/managed/stable/learn-cockroachdb-sql.html.
Please take a look at this and let me know if you need me or Amruta to fill you in on the managed side.
v19.2/learn-cockroachdb-sql.md, line 92 at r2 (raw file):
~~~ When you no longer need a table, use [`DROP TABLE`](drop-table.html) followed by the table name to remove the table and all its data:
This is going to trip up users who are following the page sequentially, as the next step depends on this new table. I'd suggest making this a new, final section called something like "Remove a table".
v19.2/learn-cockroachdb-sql.md, line 205 at r2 (raw file):
~~~ ~~~
This output isn't accurate. We haven't created a name_idx on drivers, but defining dl as UNIQUE in the table schema does mean there's an extra index here by default:
root@127.0.0.1:62104/movr> SHOW INDEX FROM drivers;
table_name | index_name | non_unique | seq_in_index | column_name | direction | storing | implicit
+------------+----------------+------------+--------------+-------------+-----------+---------+----------+
drivers | primary | false | 1 | city | ASC | false | false
drivers | primary | false | 2 | id | ASC | false | false
drivers | drivers_dl_key | false | 1 | dl | ASC | false | false
drivers | drivers_dl_key | false | 2 | city | ASC | false | true
drivers | drivers_dl_key | false | 3 | id | ASC | false | true
(5 rows)
v19.2/learn-cockroachdb-sql.md, line 316 at r2 (raw file):
{% include copy-clipboard.html %} ~~~ sql > UPDATE promo_codes SET (description, rules) = ('EXPIRED', '{"type": "percent_discount", "value": "0%"}') WHERE expiration_time < '2019-01-22 03:04:05+00:00';
This is a cool example, but I wonder if updating the JSON column is overly complex, as opposed to a simpler SQL type?
v19.2/learn-cockroachdb-sql.md, line 348 at r2 (raw file):
{% include copy-clipboard.html %} ~~~ sql > DELETE FROM promo_codes WHERE description = 'EXPIRED';
Might actually be worth showing the response here:
DELETE 669
v19.2/learn-cockroachdb-sql.md, line 353 at r2 (raw file):
{% include copy-clipboard.html %} ~~~ sql > SELECT code, description, rules FROM promo_codes LIMIT 10;
Can you help me understand how this select is a logical follow-up to the delete? I suspect there's a logic that I can't see and that might not be evident to readers.
v19.2/movr.md, line 17 at r2 (raw file):
We use sentence case in h2 headings and lower, e.g:
The
movrdatabase
Usingcockroachcommands to generate schemas and data for MovR
Please adjust these headings accordingly.
v19.2/movr.md, line 29 at r2 (raw file):
<img src="{{ 'images/v19.2/geo-partitioning-schema.png' | relative_url }}" alt="Geo-partitioning schema" style="max-width:100%" /> The `movr` database also contains the `promo_codes`, `user_promo_codes`, and `vehicle_location_histories` tables.
Feels odd that we're not listing and defining these tables in the main list and in the diagram. It'll take extra work, but it'll be worth it.
v19.2/movr.md, line 31 at r2 (raw file):
The `movr` database also contains the `promo_codes`, `user_promo_codes`, and `vehicle_location_histories` tables. ## Using `cockroach` Commands to Generate Schemas and Data for MovR
I think this heading can be shortened to Generate schemas and data for MovR
v19.2/movr.md, line 39 at r2 (raw file):
- [`cockroach workload init movr`](cockroach-workload.html) loads the `movr` database and some sample data to a running cluster. ## Using the MovR Application
This heading feels misleading, as the content is not about using the application as much as it is about understanding how it works. Can you take another shot at this?
v19.2/movr.md, line 89 at r2 (raw file):
For a tutorial about performance tuning in CockroachDB, see [Performance Tuning](performance-tuning.html). ## Related Topics
We use See also as the heading for this section throughout our docs.
v19.2/select-clause.md, line 63 at r2 (raw file):
## Examples {% include {{page.version.version}}/sql/movr-statements.md %}
Same comment as earlier: Might want to make this an h3 section, Setup.
v19.2/show-columns.md, line 41 at r2 (raw file):
## Examples {% include {{page.version.version}}/sql/movr-statements.md %}
Same comment as earlier: Might want to make this an h3 section, Setup.
v19.2/show-index.md, line 50 at r2 (raw file):
## Example {% include {{page.version.version}}/sql/movr-statements.md %}
Same comment as earlier: Might want to make this an h3 section, Setup.
v19.2/show-tables.md, line 35 at r2 (raw file):
## Examples {% include {{page.version.version}}/sql/movr-statements.md %}
Same comment as earlier: Might want to make this an h3 section, Setup.
v19.2/show-tables.md, line 107 at r2 (raw file):
{% include copy-clipboard.html %} ~~~ sql > SHOW TABLES FROM startrek.public;
This database doesn't exist if you follow our movr setup steps. Maybe show tables from the system db instead.
v19.2/show-tables.md, line 128 at r2 (raw file):
You can use [`COMMENT ON`](comment-on.html) to add comments on a table.
Add copy to clipboard.
v19.2/show-tables.md, line 135 at r2 (raw file):
To view a table's comments: ~~~ sql
Add copy to clipboard.
v19.2/update.md, line 43 at r2 (raw file):
## Examples {% include {{page.version.version}}/sql/movr-statements.md %}
Same comment as earlier: Might want to make this an h3 section, Setup.
There was a problem hiding this comment.
Reviewable status:
_includes/sidebar-data-v19.2.json, line 46 at r1 (raw file):
Previously, ericharmeling (Eric Harmeling) wrote…
Both totally legitimate reasons to just update the file with the same name rather than create an additional file and have two separate files! I'll fix this.
Done.
_includes/sidebar-data-v19.2.json, line 126 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
I don't think this is the best place for this doc. Instead, I'd recommend we create a new section under
ReferencecalledSample Applicationsor something similar, and put it there. Thoughts?
Done.... My only issue would be that MovR isn't just an application. It's mostly used as a dataset, a workload, or a database. Hence the wording "Examples" (or "Samples"), without "application". Thoughts?
_includes/v19.2/sql/movr-start.md, line 5 at r1 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
As we discussed, my vote is for simple insecure-only, since if you're using secure we can't really know what your connection string is.
Done!
_includes/v19.2/sql/movr-statements.md, line 5 at r1 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Happy to help think through conditional includes, Eric, if you'll explain more what end result you're looking for.
Sounds good, @jseldess. Rich and I agreed that in this case, the double-include works, but I am still curious to know how we handle conditional includes in Jekyll.
v19.2/cockroach-workload.md, line 90 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
What is this exactly? I'm not getting much from the current description.
I am not entirely sure what pprofport is referring to to be honest... This flag and description are taken from the output of cockroach workload init movr --help @danhhz What exactly does this flag do? Send data to some server listening at the specified port?
v19.2/create-index.md, line 54 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
This is great! What about giving this its own h3 heading like
Setupjust to make it more explicit at a TOC glance?
Done.
v19.2/create-table.md, line 148 at r1 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
Yes - I am only referring to changing the text.
Done.
v19.2/create-table.md, line 98 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
This may be an example of a page where we don't get much use of
movrsince these are all create statements. Also, if users do set up movr ahead of time, they'll end up with errors due to the names of the tables in the default dataset. Please consider removing here.
I see where you are coming from. Are you suggesting we remove just that introductory bit about MovR from create-table.md, or that we just not use MovR at all for the CREATE TABLE statement examples?
v19.2/create-table.md, line 148 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
I think the schema should be identical to the one above but with the primary key defined. The new schema adds unrelated complexity by including the check constraint. If you want to highlight check in an example, perhaps add a separate section,
Create a table with a check constraint, afterCreate a table with a foreign key constraint.
Great point. I created a new section as you suggested... I also removed that first section ("Create a table (no primary key defined)") to fix this issue: #4912.
v19.2/delete.md, line 86 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same comment as earlier: Might want to make this an h3 section,
Setup.
Done.
v19.2/drop-table.md, line 34 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same comment as earlier: Might want to make this an h3 section,
Setup.
Done.
There was a problem hiding this comment.
Reviewable status:
v19.2/cockroach-workload.md, line 90 at r2 (raw file):
Previously, ericharmeling (Eric Harmeling) wrote…
I am not entirely sure what
pprofportis referring to to be honest... This flag and description are taken from the output ofcockroach workload init movr --help@danhhz What exactly does this flag do? Send data to some server listening at the specified port?
It's used for performance profiling of workload itself and should probably be hidden from users. mind filing an issue?
There was a problem hiding this comment.
Reviewable status:
v19.2/cockroach-workload.md, line 90 at r2 (raw file):
Previously, danhhz (Daniel Harrison) wrote…
It's used for performance profiling of workload itself and should probably be hidden from users. mind filing an issue?
Sure thing!
v19.2/delete.md, line 137 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
If you followed the first example, you're not going to get any results here. Would it be possible to make these examples operate on different segments of data? Maybe the first example should delete from a different table, for example.
Great point. Done.
v19.2/delete.md, line 137 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Hmm, this pre-dates your changes, but I wonder if we should remove the
RETURNINGclause from this and the next query since returning is the focus of the next section?
Done.
v19.2/insert.md, line 584 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
No rows match this condition at this point, so you might want to rethink this a bit.
I updated the user_id value on this example and the following examples to match one of the preloaded movr workload user_id values from select * from users where city='new york';. This should work now.
v19.2/learn-cockroachdb-sql.md, line 10 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
This logic was in place to create slightly different outputs for regular CockroachDB docs vs the same page in the Managed CockroachDB docs. As is, the new version does not work for Managed CockroachDB. Compare http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/0cf4fe269ac9d714ad93eb0d5e01eeed96809384/managed/dev/learn-cockroachdb-sql.html to http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/0cf4fe269ac9d714ad93eb0d5e01eeed96809384/managed/stable/learn-cockroachdb-sql.html.
Please take a look at this and let me know if you need me or Amruta to fill you in on the managed side.
OK! I might need to talk to you about this offline... I'm not seeing any managed docs that talk about specific workloads or demo datasets, other than the defaultdb, and for any of the movr examples to make sense, that dataset needs to be preloaded.
v19.2/learn-cockroachdb-sql.md, line 92 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
This is going to trip up users who are following the page sequentially, as the next step depends on this new table. I'd suggest making this a new, final section called something like "Remove a table".
Great idea. Done.
v19.2/learn-cockroachdb-sql.md, line 205 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
This output isn't accurate. We haven't created a
name_idxondrivers, but definingdlasUNIQUEin the table schema does mean there's an extra index here by default:root@127.0.0.1:62104/movr> SHOW INDEX FROM drivers; table_name | index_name | non_unique | seq_in_index | column_name | direction | storing | implicit +------------+----------------+------------+--------------+-------------+-----------+---------+----------+ drivers | primary | false | 1 | city | ASC | false | false drivers | primary | false | 2 | id | ASC | false | false drivers | drivers_dl_key | false | 1 | dl | ASC | false | false drivers | drivers_dl_key | false | 2 | city | ASC | false | true drivers | drivers_dl_key | false | 3 | id | ASC | false | true (5 rows)
I'm just going to remove this, since we already have a statement for the users table.
v19.2/learn-cockroachdb-sql.md, line 316 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
This is a cool example, but I wonder if updating the JSON column is overly complex, as opposed to a simpler SQL type?
I see what you mean. It is a little complex... and there might be other areas where we can showcase JSON support. Let's talk about this too.
v19.2/learn-cockroachdb-sql.md, line 348 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Might actually be worth showing the response here:
DELETE 669
Done.
v19.2/learn-cockroachdb-sql.md, line 353 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Can you help me understand how this select is a logical follow-up to the delete? I suspect there's a logic that I can't see and that might not be evident to readers.
It was really just to show that all of the deleted rows are no longer in the table. I can remove this though, in favor of using the DELETE response above.
v19.2/movr.md, line 17 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
We use sentence case in h2 headings and lower, e.g:
The
movrdatabase
Usingcockroachcommands to generate schemas and data for MovRPlease adjust these headings accordingly.
Done.
v19.2/movr.md, line 29 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Feels odd that we're not listing and defining these tables in the main list and in the diagram. It'll take extra work, but it'll be worth it.
I agree. As you mentioned when we met last week, the MovR dataset contained just these three "main" tables when you created the geo-partitioning tutorial. Let's talk about this offline - I want to know how you made your diagrams (tooling, style/color guide, etc.). I'm going to remove the diagram for now, make this list a table, and make sure that table includes all tables in the dataset.
v19.2/movr.md, line 31 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
I think this heading can be shortened to
Generate schemas and data for MovR
Done!
v19.2/movr.md, line 39 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
This heading feels misleading, as the content is not about using the application as much as it is about understanding how it works. Can you take another shot at this?
Sure thing... I'm thinking "How the MovR application works" is pretty to-the-point.
v19.2/movr.md, line 89 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
We use
See alsoas the heading for this section throughout our docs.
Done.
v19.2/select-clause.md, line 63 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same comment as earlier: Might want to make this an h3 section,
Setup.
I've added this to the movr-statements.md file itself.
v19.2/show-columns.md, line 41 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same comment as earlier: Might want to make this an h3 section,
Setup.
I've added this to the movr-statements.md file itself.
v19.2/show-index.md, line 50 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same comment as earlier: Might want to make this an h3 section,
Setup.
I've added this to the movr-statements.md file itself.
v19.2/show-tables.md, line 35 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same comment as earlier: Might want to make this an h3 section,
Setup.
I've added this to the movr-statements.md file itself.
v19.2/show-tables.md, line 107 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
This database doesn't exist if you follow our movr setup steps. Maybe show tables from the
systemdb instead.
Done.
v19.2/show-tables.md, line 128 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Add copy to clipboard.
Done.
v19.2/show-tables.md, line 135 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Add copy to clipboard.
Done.
v19.2/update.md, line 43 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same comment as earlier: Might want to make this an h3 section,
Setup.
I've added this to the movr-statements.md file itself.
ericharmeling
force-pushed
the
movr-update
branch
from
0cf4fe2 to
b538364
Compare
ericharmeling
force-pushed
the
movr-update
branch
from
d41b916 to
b1544f5
Compare
There was a problem hiding this comment.
This LGTM now, once we figure out the managed vs. non-managed situation for learn cockroachdb.
Reviewed 1 of 23 files at r1, 1 of 14 files at r2, 2 of 10 files at r3.
Reviewable status:
_includes/sidebar-data-v19.2.json, line 126 at r2 (raw file):
Previously, ericharmeling (Eric Harmeling) wrote…
Done.... My only issue would be that MovR isn't just an application. It's mostly used as a dataset, a workload, or a database. Hence the wording "Examples" (or "Samples"), without "application". Thoughts?
Valid point, but despite it also being a static dataset, I think "Sample Applications" focuses on the fact that it is based on a fictional application. I like it how you have it, and I like imagining that we might add more sample applications as well. Maybe show it in the sidenav as `MovR: vehicle-sharing app" or something?
v19.2/create-table.md, line 98 at r2 (raw file):
Previously, ericharmeling (Eric Harmeling) wrote…
I see where you are coming from. Are you suggesting we remove just that introductory bit about MovR from
create-table.md, or that we just not use MovR at all for theCREATE TABLEstatement examples?
Yes, the former.
v19.2/learn-cockroachdb-sql.md, line 10 at r2 (raw file):
Previously, ericharmeling (Eric Harmeling) wrote…
OK! I might need to talk to you about this offline... I'm not seeing any managed docs that talk about specific workloads or demo datasets, other than the
defaultdb, and for any of themovrexamples to make sense, that dataset needs to be preloaded.
This is the one remaining blocker to this PR. Let's talk about this in person tomorrow. We might need to just split this into 2 pages, one for managed and one for non-managed.
v19.2/movr.md, line 29 at r2 (raw file):
Previously, ericharmeling (Eric Harmeling) wrote…
I agree. As you mentioned when we met last week, the MovR dataset contained just these three "main" tables when you created the geo-partitioning tutorial. Let's talk about this offline - I want to know how you made your diagrams (tooling, style/color guide, etc.). I'm going to remove the diagram for now, make this list a table, and make sure that table includes all tables in the dataset.
I can point you to the diagrams, but for now, I think your table is perfect.
|
v19.2/movr.md, line 13 at r3 (raw file):
On another review, I think this first bullet isn't necessary. Focusing on this as a dataset, which implies a database, and an application, seems clearer. |
|
_includes/sidebar-data-v19.2.json, line 126 at r2 (raw file): Previously, jseldess (Jesse Seldess) wrote…
Thinking about this more. We could also name this section just "Workloads" and consider featuring more of all of the workloads and datasets built into |
|
@ericharmeling, teamcity is failing due to these broken links: Let me know if you need any help resolving them. |
There was a problem hiding this comment.
Reviewable status:
_includes/sidebar-data-v19.2.json, line 126 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Thinking about this more. We could also name this section just "Workloads" and consider featuring more of all of the workloads and datasets built into
cockroach workloadandcockroach demo(separately, of course). I'm thinking YCSB and TPCC would be particularly useful to call out as workloads. Thoughts?
I changed the navbar entry for MovR, as suggested.
Also, we talked about this offline earlier... Let's stick with "Sample Applications" for the MovR overview for now. As we add more sample applications, we can describe them at a high level in this section as well. We can make a "Workloads" section separate from that. I'm going to open a docs project issue about creating that "Workloads" section.
v19.2/create-table.md, line 98 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Yes, the former.
Done.
v19.2/learn-cockroachdb-sql.md, line 10 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
This is the one remaining blocker to this PR. Let's talk about this in person tomorrow. We might need to just split this into 2 pages, one for managed and one for non-managed.
I opened a separate PR for this... #5216
v19.2/movr.md, line 29 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
I can point you to the diagrams, but for now, I think your table is perfect.
Right on... I'll probably open a separate issue about improving/expanding the "Example Applications" overview.
v19.2/movr.md, line 13 at r3 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
On another review, I think this first bullet isn't necessary. Focusing on this as a dataset, which implies a database, and an application, seems clearer.
Done.
ericharmeling
force-pushed
the
movr-update
branch
from
b1544f5 to
ae841b9
Compare
SQL Statements examples: first commit (SHOW TABLES, CREATE TABLE, other markdown file dependencies) Added MovR overview doc; Removed drivers from two examples Added movr command to demo and workload More SQL statement examples (SHOW COLUMNS, SHOW TABLES, DROP TABLE) More SQL statement examples (INSERT) More SQL statement examples (DELETE, UPDATE, SELECT) Follow-up on rmloveland comments Follow-up on jseldess comments Removed MovR Learn CRDB SQL from movr-update feature branch
ericharmeling
force-pushed
the
movr-update
branch
from
ae841b9 to
d5a9ad3
Compare
Fixed the broken links... These happened because I removed the |
This is the very first round of
movrdoc updates. These changes are all for 19.2 doc.This PR includes the following:
movrcommand added to thecockroach demoandcockroach workloadcommand documentation for 19.2 (cockroach-demo.md, cockroach-workload.md) Update demo and workload flags for new workload(s) in 19.2 #5062movrdatabase instead of other generic examples (learn-cockroachdb-sql-movr.md) Learn CockroachDB SQL with MovR #5040Note that I've also changed/added several other include files where necessary. I suspect there are plenty of changes that need to be made to this PR upon review. No rush!