Skip to content

Conversation

@srutzky
Copy link
Contributor

@srutzky srutzky commented Jun 22, 2020

This fixes #5007

  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 first 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 (query to disable all DML triggers) as the variable is NVARCHAR and MS documentation should show best practices.
  9. Added link to "Logon Triggers" in "See Also" section.

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.
@PRMerger7
Copy link
Contributor

@srutzky : Thanks for your contribution! The author(s) have been notified to review your proposed change.

@ktoliver ktoliver added the aq-pr-triaged tracking label for the PR review team label Jun 22, 2020
@WilliamAntonRohm
Copy link
Contributor

@rothja -- Jason, please review these changes.

@srutzky srutzky changed the title Formatting fixes for "Manager Trigger Security" Formatting fixes for "Manage Trigger Security" Jun 22, 2020
@rothja
Copy link
Collaborator

rothja commented Jun 23, 2020

@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.

@PRMerger13
Copy link
Contributor

pmasl. is not a valid GitHub ID, or is not a collaborator on this repo.

@rothja
Copy link
Collaborator

rothja commented Jun 23, 2020

Note: I'm contacting Pedro offline. Thanks!

@rothja
Copy link
Collaborator

rothja commented Jul 2, 2020

@srutzky These changes have been reviewed/approved. I will look at the merge conflicts to resolve and sign off.

@PRMerger17
Copy link
Contributor

@rothja : Thanks for your contribution! The author(s) have been notified to review your proposed change.

@rothja
Copy link
Collaborator

rothja commented Jul 2, 2020

#sign-off

@rothja rothja merged commit 8ad4aca into MicrosoftDocs:live Jul 2, 2020
@srutzky
Copy link
Contributor Author

srutzky commented Jul 2, 2020

@rothja Do you want me to fix something?

@rothja
Copy link
Collaborator

rothja commented Jul 2, 2020

@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!

@srutzky
Copy link
Contributor Author

srutzky commented Jul 2, 2020

@rothja Yes, that's perfect. I see now the conflict you were talking about. Looks good.

@srutzky srutzky deleted the patch-6 branch October 22, 2021 16:34
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.

Security scenario

9 participants