Skip to content

[New Entry] SQL Commands: SELECT TOP #1305

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

Merged
merged 13 commits into from
Jan 30, 2023
Merged

[New Entry] SQL Commands: SELECT TOP #1305

merged 13 commits into from
Jan 30, 2023

Conversation

jmpos7
Copy link
Contributor

@jmpos7 jmpos7 commented Dec 7, 2022

Description

Adding a basic new entry for the TOP command, which did not have an entry. Adding to the Codecademy documentation is a project in the Learn Git & GitHub course and I wanted to make a contribution.

Closes #1303

Type of Change

  • Adding a new entry
  • Updating the documentation

Checklist

  • All writings are my own.
  • My entry follows the Codecademy Docs style guide.
  • My changes generate no new warnings.
  • I have performed a self-review of my own writing and code.
  • I have checked my entry and corrected any misspellings.
  • I have made corresponding changes to the documentation if needed.
  • I have confirmed my changes are not being pushed from my forked main branch.
  • I have confirmed that I'm pushing from a new branch named after the changes I'm making.
  • Under "Development" on the right, I have linked any issues that are relevant to this PR (write "Closes # in the "Description" above).

@CLAassistant
Copy link

CLAassistant commented Dec 7, 2022

CLA assistant check
All committers have signed the CLA.

@SSwiniarski SSwiniarski added sql SQL entries new entry New entry or entries labels Dec 7, 2022
@SSwiniarski
Copy link
Contributor

@jmpos7, thanks for your contribution. Please remember to sign the CLA.

@Dusch4593
Copy link
Contributor

Hi @jmpos7, wanted to also confirm that this pull request aims to fix issue #1303?

@jmpos7
Copy link
Contributor Author

jmpos7 commented Dec 7, 2022

@Dusch4593 Confirmed

@Dusch4593
Copy link
Contributor

@jmpos7 awesome thanks! And double-thanks for signing the CLA 👍🏻

@Dusch4593 Dusch4593 changed the title Top [New Entry] SQL Commands: SELECT TOP Dec 8, 2022
Copy link
Contributor

@SSwiniarski SSwiniarski left a comment

Choose a reason for hiding this comment

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

@jmpos7 , I did a first review and left one comment to address.


## Syntax

```sql
Copy link
Contributor

Choose a reason for hiding this comment

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

The syntax section needs a few changes:

  • The examples should be in a ```pseudo block.
  • The examples should be generalized, such as SELECT TOP (n) SELECT TOP (n) PERCENT
  • Each example should have an explanatory sentence describing what it's doing and what n is.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would also include a sentence in the "Syntax" section that explains how column_name(s) is a space-separated list of existing columns that are selected from table_name.

@Dusch4593
Copy link
Contributor

Dusch4593 commented Dec 20, 2022

Hey @jmpos7! 👋🏻 Did you have any questions about or need any assistance with implementing the feedback?

@jmpos7
Copy link
Contributor Author

jmpos7 commented Jan 7, 2023

@Dusch4593 Thanks for checking in -- I made the reviewer recommended changes to the document. What is the next step?

Copy link
Contributor

@SSwiniarski SSwiniarski left a comment

Choose a reason for hiding this comment

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

@jmpos7 I had two more minor changes, but otherwise looks good for a second review.

Copy link
Contributor

@Dusch4593 Dusch4593 left a comment

Choose a reason for hiding this comment

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

@jmpos7 Left you some comments/suggestions but I may have more after this current feedback is implemented. 😄

@@ -0,0 +1,48 @@
---
Title: 'TOP'
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if this entry should be repurposed as a concept entry and renamed as 'SELECT TOP' (similar to the existing SELECT DISTINCT entry).

cc: @SSwiniarski


## Syntax

```sql
Copy link
Contributor

Choose a reason for hiding this comment

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

I would also include a sentence in the "Syntax" section that explains how column_name(s) is a space-separated list of existing columns that are selected from table_name.

SSwiniarski and others added 2 commits January 30, 2023 11:43
Co-authored-by: Brandon Dusch <brandondusch@gmail.com>
Co-authored-by: Brandon Dusch <brandondusch@gmail.com>
@SSwiniarski SSwiniarski dismissed Dusch4593’s stale review January 30, 2023 16:44

@Dusch4593 is no longer a maintainer on this repro.

@SSwiniarski
Copy link
Contributor

@jmpos7 because there's been over two weeks of inactivity, an In the interests of merging this PR, I went ahead an committed all of @Dusch4593's suggestions.

@SSwiniarski SSwiniarski merged commit adbfdd3 into Codecademy:main Jan 30, 2023
@github-actions
Copy link

👋 @jmpos7
You have contributed to Codecademy Docs, and we would like to know more about you and your experience.
Please take a minute to fill out this four question survey to help us better understand Docs contributions and how we can improve the experience for you and our learners.
Thank you for your help!

@SSwiniarski
Copy link
Contributor

Congrats @jmpos7! Your new entry is live here: https://www.codecademy.com/resources/docs/sql/commands/select-top

It looks like you're currently anonymous.
image
If you'd like to become public, make sure to add your GitHub username to your Codecademy profile and set your visibility to everyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Entry] SQL: SELECT TOP
4 participants