-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Formatting fixes for "Manage Trigger Security" #5026
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
1. Changed formatting of user name "JohnDoe" in first and second paragraphs to bold instead of inline-code to be consistent with rest of formatting in that paragraph. 2. Fixed example code formatting to be in a fenced code block instead of each line being inline-code. 3. Fixed exploit example code to be realistic _and_ work outside of the `[master]` database. 4. Removed extraneous space(s) at the end of most lines within the fenced code blocks (fenced code blocks preserve whitespace and hence extra spaces at the end of lines provides no markdown rendering benefit, but they do make it messier when readers copy and paste to run it locally). 5. Removed extraneous space at the end of most lines, just before the semicolon (just looks like poor formatting). 6. Changed `UNION` to `UNION ALL` in second fenced code block (best practice). 7. Made all fenced code blocks use "sql" style whereas previously only one of them used it and the rest did not have a style. I could not find any consistency across several other pages that I just checked for reference, so I erred on the side of readability (and what I, as a reader, prefer). 8. Added uppercase-"N" prefix to string literals in final fenced code block as the variable is NVARCHAR and MS documentation should show best practices. 9. Added link to "Logon Triggers" in "See Also" section.
|
@srutzky : Thanks for your contribution! The author(s) have been notified to review your proposed change. |
|
@rothja -- Jason, please review these changes. |
|
@srutzky This was a lot of work making the article better. Thank you! I want to get one other person to verify the changes before we publish. @pmasl Could you please take look at the changes proposed here to the "Manage Trigger Security" article. See the file changes in this PR. Note that Solomon has graciously offered to suggest the changes to address another customer issue here: .#5007. #assign:pmasl. Pedro, if you think someone else should review, please let me know. Thank you. |
|
pmasl. is not a valid GitHub ID, or is not a collaborator on this repo. |
|
Note: I'm contacting Pedro offline. Thanks! |
|
@srutzky These changes have been reviewed/approved. I will look at the merge conflicts to resolve and sign off. |
|
@rothja : Thanks for your contribution! The author(s) have been notified to review your proposed change. |
|
#sign-off |
|
@rothja Do you want me to fix something? |
|
@srutzky No. We changed how we do the "Applies to" at the top, so it caused a merge conflict. I fixed it for you and pushed the changes to your branch. I hope that is OK. I merged these changes in along with my conflict fix. Thank you! |
|
@rothja Yes, that's perfect. I see now the conflict you were talking about. Looks good. |
This fixes #5007
[master]database.UNIONtoUNION ALLin second fenced code block (best practice).Nprefix to string literals in final fenced code block (query to disable all DML triggers) as the variable isNVARCHARand MS documentation should show best practices.