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

Docs: Fix missing semicolons in SQL snippets. #8748

Merged
merged 3 commits into from
Oct 10, 2023

Conversation

Priyansh121096
Copy link
Contributor

Add a missing semicolon to the "CREATE TABLE ..." statement.

Add a missing semicolon to the "CREATE TABLE ..." statement.
@github-actions github-actions bot added the docs label Oct 8, 2023
@Priyansh121096 Priyansh121096 changed the title Update spark-getting-started.md Docs: Update spark-getting-started.md Oct 8, 2023
Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to fix this @Priyansh121096

@Priyansh121096
Copy link
Contributor Author

No worries @Fokko. Stumbled upon this while blindly copy pasting from the getting started page and thought I'd put in a fix.

Btw, I've also raised apache/iceberg-docs#281. I'm not sure which is the one that actually gets deployed to https://iceberg.apache.org/docs/latest/getting-started/.

@@ -69,7 +69,7 @@ To create your first Iceberg table in Spark, use the `spark-sql` shell or `spark

```sql
-- local is the path-based catalog defined above
CREATE TABLE local.db.table (id bigint, data string) USING iceberg
CREATE TABLE local.db.table (id bigint, data string) USING iceberg;
Copy link
Member

Choose a reason for hiding this comment

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

Can you please look up ```sql in the code and fix all the places? I found few more places in the docs like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ajantha-bhat, this is done.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would be even better to have some kind of linter for this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Fokko Agreed.

@ajantha-bhat ajantha-bhat self-requested a review October 9, 2023 05:50
@ajantha-bhat
Copy link
Member

ajantha-bhat commented Oct 9, 2023

Btw, I've also raised apache/iceberg-docs#281. I'm not sure which is the one that actually gets deployed to https://iceberg.apache.org/docs/latest/getting-started/.

.md files are modified in this repo and referred from iceberg-docs repo. So, fixing here is enough.

Thumb rule is that if you find the docs in iceberg repo, fix here. If the doc is present only in iceberg-docs repo. Fix only in iceberg-doc repo.

@Priyansh121096 Priyansh121096 changed the title Docs: Update spark-getting-started.md Docs: Fix missing semicolons in SQL snippets. Oct 9, 2023
Copy link
Member

@ajantha-bhat ajantha-bhat left a comment

Choose a reason for hiding this comment

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

LGTM. It is good to have this.

Even examples in Spark docs (https://spark.apache.org/docs/latest/sql-ref-syntax.html) follows semicolon syntax

@Fokko
Copy link
Contributor

Fokko commented Oct 10, 2023

Great work @Priyansh121096 If we find more we can create a new PR. (I also noticed that some blocks start with:

```SQL

For consistency it would be nice to have everything lowercase, but I think that works as well.

@Fokko Fokko merged commit 982242b into apache:master Oct 10, 2023
2 checks passed
@Priyansh121096
Copy link
Contributor Author

Thanks @Fokko! This was my first contribution to apache/iceberg. Hope to make many more in the future.

@Priyansh121096 Priyansh121096 deleted the patch-1 branch October 31, 2023 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants