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

Learn CRDB SQL MovR update #5216

Merged
merged 1 commit into from
Aug 15, 2019

Conversation

ericharmeling
Copy link
Contributor

@ericharmeling ericharmeling commented Aug 13, 2019

#5040
This PR includes the "Learn CockroachDB SQL" MovR update, originally a part of the #5075 PR. I broke it out into a different feature branch because of the conflicts with MSO...

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Added managed version of Learn CRDB SQL
@ericharmeling
Copy link
Contributor Author

@jseldess As you can see in the file diff, I crammed in two separate Learn CRDB SQL instructions... One that uses MovR, for the standard 19.2 docs, and one that does not use MovR (it is essentially identical to the 19.1 Learn CRDB SQL page), for managed.

Copy link
Contributor

@jseldess jseldess left a comment

Choose a reason for hiding this comment

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

:lgtm:

There's an argument to be made that, since no content on the page is really shared anymore, it might be fine to split this into 2 files, e.g., learn-cockroachdb-sql.md and managed-learn-cockroachdb.sql.md. But let's leave it as a single file for now.

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


v19.2/learn-cockroachdb-sql.md, line 20 at r1 (raw file):

Do one of the following:

{% include {{page.version.version}}/sql/movr-start.md %}

I have a few ideas about improving this include, but I'll show you in a separate PR.

@jseldess jseldess merged commit c9bcc3d into cockroachdb:master Aug 15, 2019
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