Skip to content
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

Fix #232 (report appears if all tables are there) #247 (fix working set trimming report) and #243 (add CCC rule) #252

Merged
merged 8 commits into from
Sep 13, 2023

Conversation

hacitandogan
Copy link
Contributor

worked on the issue #232
Although this should prevent those errors , for future maybe we should consider some of our reports to handle these kind of situations.(Especially when the partial information is still meaningfull even if you have some data missing )

…n SQL Perf Main report when not all tables are present

worked on the microsoft#232
some Microsoft modules uses  company name "Microsoft Corp." instead of "Microsoft Corporation"  , no need to highlight them.
Added an additional if condition to the background color field.
Just removed the field "Indicators"
Should we add [IndicatorsProcess] ,[IndicatorsSystem] ,[IndicatorsPool]  instead ?
Added [IndicatorsProcess] ,[IndicatorsSystem] ,[IndicatorsPool] as part of discussion under microsoft#247.
added rule "Common Criteria Compliance Enabled" and stored procedure usp_CCC_Enabled for the Rule
@PiJoCoder PiJoCoder changed the title #232 If a report uses multiple tables it should not appear on SQL Perf Main report when not all tables are present Fix #232 (report appears if all tables are there) #247 (fix working set trimming report) and #243 (add CCC rule) Sep 12, 2023
@PiJoCoder
Copy link
Contributor

@hacitandogan thank you for all this work. Just saw this today. I'll be testing and providing feedback. Thank you

@PiJoCoder
Copy link
Contributor

@hacitandogan thank you

  • Tested working set trimming report - looks good
  • Tested multiple tables report - tempdb space usage - looks good
  • For usp_CCC_Enabled, add an "EXEC usp_CCC_Enabled" at the bottom of PerfStatsAnalysis.sql to activate the rule
  • Could you undo the changes to NexusReports/NexusReports.rptproj.rsuser it gets automatically modified every time you make a report change and I personally reverse the change to it as this change is not relevant. There may be a way to disable changing the user's property here, but I have investigated. For now, just dicard the change to this file and push
  • Can you do a git pull upstream master since we merged a PR today and the Master is updated and push

update NexusReports\NexusReports.rptproj.rsuser to revert it back to original state
added missing EXEC statement for activating the rule.
@hacitandogan
Copy link
Contributor Author

@PiJoCoder
Thank you.
I did the modifications and tried to revert .rsuser file to its original yet it still looks like a commit , could you please review and comment ?

@PiJoCoder
Copy link
Contributor

@PiJoCoder Thank you. I did the modifications and tried to revert .rsuser file to its original yet it still looks like a commit , could you please review and comment ?

Looks like it worked. I don't see the file listed as one of the changed ones:
image

@PiJoCoder
Copy link
Contributor

Tested Common criteria compliance rule - looks good
image

Copy link
Contributor

@PiJoCoder PiJoCoder left a comment

Choose a reason for hiding this comment

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

Looks great

@PiJoCoder PiJoCoder merged commit 28c6265 into microsoft:master Sep 13, 2023
@PiJoCoder
Copy link
Contributor

Thank you @hacitandogan - great work on these!

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

Successfully merging this pull request may close these issues.

2 participants