-
Notifications
You must be signed in to change notification settings - Fork 4.1k
[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
Conversation
@Dusch4593 Confirmed |
@jmpos7 awesome thanks! And double-thanks for signing the CLA 👍🏻 |
There was a problem hiding this 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
.
Hey @jmpos7! 👋🏻 Did you have any questions about or need any assistance with implementing the feedback? |
@Dusch4593 Thanks for checking in -- I made the reviewer recommended changes to the document. What is the next step? |
There was a problem hiding this 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.
There was a problem hiding this 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' |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
.
Co-authored-by: Brandon Dusch <brandondusch@gmail.com>
Co-authored-by: Brandon Dusch <brandondusch@gmail.com>
Co-authored-by: Brandon Dusch <brandondusch@gmail.com>
Co-authored-by: Brandon Dusch <brandondusch@gmail.com>
@Dusch4593 is no longer a maintainer on this repro.
…ontent/sql/concepts/commands/terms/select-top/select-top.md
@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. |
👋 @jmpos7 |
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. |
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
Checklist
main
branch.