Trivadis/plsql-and-sql-coding-guidelinesPublic

NotificationsYou must be signed in to change notification settings
Fork76
Star191

Changes for version 4.0#86

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

kibeha merged 19 commits intoTrivadis:mainfromkibeha:main

Sep 23, 2020

Merged

Changes for version 4.0#86

kibeha merged 19 commits intoTrivadis:mainfromkibeha:main

Sep 23, 2020

Conversation

Copy link

Collaborator

kibeha commentedSep 17, 2020

Closes#22
Closes#25
Closes#27
Closes#30
Closes#31
Closes#38
Closes#44
Closes#45
Closes#48
Closes#57
Closes#77
Closes#78
Closes#79
Closes#81
Closes#84
Closes#85

kibeha added19 commits

August 26, 2020 09:05

FixTrivadis#81- Removed G-7240 and replaced with G-7160 and G-7170

2a0b3a2

FixTrivadis#22- Added Transaction Control category with rules G-321…

efe7c60

…0 and G-3220

New Trivadis style for PDF cover. Slight addition to introduction.

29d1079

Use of codehilite markdown extension for syntax highlighting and line numbers of SQL & PL/SQL.

FixTrivadis#22- Re-numbered falsely created rules G-3210 and G-3220…

81c2836

… to G-3310 and G-3320. Fixed links for these rules in PDF.

FixTrivadis#57- Altered rule to all lowercase. Added footnote on re…

ca35d65

…ason and importance of consistency. Converted all code examples to lowercase (thanks Philipp forhttps://www.salvis.com/blog/2020/08/28/formatting-sql-code-blocks-in-markdown-files/ )

FixTrivadis#25- Created rule G-7240: Never use RETURN in package in…

5607a7c

…itialization block.

FixTrivadis#27- Created rule G-7720: Never use multiple UPDATE OF i…

e414069

…n trigger event clause.

FixTrivadis#31- Created rule G-4250: Avoid using identical conditio…

bc849ca

…ns in different branches of the same IF or CASE statement.

FixTrivadis#85- Warn about NULL issues in rule G-4220

9d618b5

FixTrivadis#38- Created rule G-3195: Always use wildcards in a LIKE…

c2f4762

… clause.

FixTrivadis#57- Lowercasing inline keywords in text and a few code …

7a6c975

…samples that automatic script didn't handle. Fixed a few typos along the way.

FixTrivadis#30- Created rule G-7330: Always assign values to OUT pa…

5af501b

…rameters.

FixTrivadis#48- Created rule G-2135: Avoid assigning values to loca…

e10f537

…l variables that are not used by a subsequent statement.

FixTrivadis#84- Created rule G-7730: Avoid multiple DML events per …

865aac4

…trigger if primary key is assigned in trigger.

FixTrivadis#44- Created rule G-4260: Avoid inverting boolean condit…

16d67a4

…ions.

FixTrivadis#45- Created rule G-4270: Avoid comparing boolean values…

cb2ff5b

… to boolean literals.

FixTrivadis#79- Added phrasing and slight reworking of sample code …

93abd1b

…in G-7230 concerning public constants.

FixTrivadis#78- Added paragraph explaining that exceptions to Rule …

9d2d2f5

…G-1050 exist (like Logger)

FixTrivadis#77- Added explanation about exception to rule G-5040 in…

e2c82be

… case of proper use of Logger.

kibeha added this to thev4.0 milestone

Sep 17, 2020

kibeha requested a review fromPhilippSalvisberg

September 17, 2020 10:06

PhilippSalvisberg reviewed

Sep 17, 2020

View reviewed changes

docs/3-coding-style/coding-style.mdShow resolvedHide resolved

PhilippSalvisberg reviewed

Sep 17, 2020

View reviewed changes

docs/4-language-usage/1-general/g-1050.mdShow resolvedHide resolved

PhilippSalvisberg reviewed

Sep 17, 2020

View reviewed changes

docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4260.mdShow resolvedHide resolved

PhilippSalvisberg reviewed

Sep 17, 2020

View reviewed changes

docs/4-language-usage/7-stored-objects/1-general/g-7170.mdShow resolvedHide resolved

PhilippSalvisberg reviewed

Sep 17, 2020

View reviewed changes

Copy link

Collaborator

PhilippSalvisberg left a comment

Choose a reason for hiding this comment

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

I think !!! missing "Unsupported in PL/SQL Cop Validators" should be added to introduction.md. Probably not under "Severity of the rule". Maybe an addition chapter.

PhilippSalvisberg reviewed

Sep 17, 2020

View reviewed changes

Copy link

Collaborator

PhilippSalvisberg left a comment

Choose a reason for hiding this comment

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

The following rules are currently not supported by a PL/SQL Cop validator:

G-3160 (cannot identify type of a column)
G-5010 (cannot identify logging framework and where it should be applied)
G-8410 (cannot identify where an application lock would make sense)
G-8510 (cannot know where the use of application_info is sensible)

They should be tagged with "!!! missing "Unsupported in PL/SQL Cop Validators"

PhilippSalvisberg reviewed

Sep 17, 2020

View reviewed changes

docs/4-language-usage/7-stored-objects/1-general/g-7170.mdShow resolvedHide resolved

PhilippSalvisberg reviewed

Sep 17, 2020

View reviewed changes

docs/4-language-usage/7-stored-objects/7-triggers/g-7730.mdShow resolvedHide resolved

PhilippSalvisberg reviewed

Sep 17, 2020

View reviewed changes

docs/4-language-usage/3-dml-and-sql/1-general/g-3190.mdShow resolvedHide resolved

PhilippSalvisberg reviewed

Sep 17, 2020

View reviewed changes

Copy link

Collaborator

PhilippSalvisberg left a comment

Choose a reason for hiding this comment

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

The table B in the appendix.md should be updated.

changed rule numbers
new rules

PhilippSalvisberg approved these changes

Sep 17, 2020

View reviewed changes

Copy link

Collaborator

PhilippSalvisberg left a comment

Choose a reason for hiding this comment

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

Great work. Thanks Kim!

I scanned the following

each changed file
browsed through every HTML page
browsed through the complete PDF

All of my comments are suggestions. Feel free to ignore them or apply them later. Up to you. You may merge this PR whenever you are ready.

There were a lot of changes due to the change of keywords from uppercase to lowercase. And not all codes were changed automatically. And finding the missing ones was possible only with looking at the source/result manually.

Thanks again.
Philipp