diff --git a/docs/reference/release-notes.md b/docs/reference/release-notes.md index 9e272893..6949f5d9 100644 --- a/docs/reference/release-notes.md +++ b/docs/reference/release-notes.md @@ -3,6 +3,13 @@ Sigrid release notes SIG uses [continuous delivery](https://en.wikipedia.org/wiki/Continuous_delivery), meaning that every change to Sigrid or the underlying analysis is released once our development pipeline has completed. On average, we release somewhere between 10 and 20 times per day. This page therefore doesn't list every single change, since that would quickly lead to an excessively long list of small changes. Instead, this page lists Sigrid and analysis changes that we consider noteworthy for the typical Sigrid user. +### November 4, 2024 + +- **Objectives:** The portfolio objectives dashboard now allows you to drag-and-drop objectives to indicate precedence. +- **Code Explorer:** All files in the system are now shown by the Code Explorer. Previously, only files in scope for the maintainability rating were shown. +- **Maintainability:** Introduced a new sytem-level maintainability dashboard, that includes several new visualization. +- **Sigrid CI:** Tweaked pull request feedback to avoid it becoming excessively long when changing files that contain a lot of technical debt. + ### October 21, 2024 - **Open Source Health:** Removed dependencies are now depicted as "grayed out" in the list of open source dependencies. This helps to avoid confusion, as people might inadvertently think those dependencies are still in use. diff --git a/sigridci/sigridci/markdown_report.py b/sigridci/sigridci/markdown_report.py index 2e914be7..18764702 100644 --- a/sigridci/sigridci/markdown_report.py +++ b/sigridci/sigridci/markdown_report.py @@ -16,10 +16,14 @@ import os from .objective import Objective, ObjectiveStatus +from .platform import Platform from .report import Report class MarkdownReport(Report): + ALLOW_FANCY_MARKDOWN = True + MAX_SHOWN_FINDINGS = 8 + MAX_OCCURRENCES = 3 RISK_CATEGORY_SYMBOLS = { "VERY_HIGH" : "🔴", @@ -38,50 +42,57 @@ def renderMarkdown(self, analysisId, feedback, options): status = Objective.determineStatus(feedback, options) sigridLink = self.getSigridUrl(options) - md = "# Sigrid maintainability feedback\n\n" + md = f"# [Sigrid]({sigridLink}) maintainability feedback\n\n" md += f"{self.renderSummary(feedback, options)}\n\n" - md += f"Sigrid compared your code against the baseline of {self.formatBaseline(feedback)}.\n\n" if status != ObjectiveStatus.UNKNOWN: - md += self.renderRefactoringCandidates(feedback, options) - md += "## Sigrid ratings\n\n" + if self.isHtmlMarkdownSupported(): + md += "
Show details\n\n" + + md += f"Sigrid compared your code against the baseline of {self.formatBaseline(feedback)}.\n\n" + md += self.renderRefactoringCandidates(feedback, sigridLink) + md += "## ⭐️ Sigrid ratings\n\n" md += self.renderRatingsTable(feedback) - md += "\n----\n\n" - md += f"[**View this system in Sigrid**]({sigridLink})\n" - if options.feedbackURL and status != ObjectiveStatus.UNKNOWN: - md += "\n----\n\n" - md += "## Did you find this feedback helpful?\n\n" - md += "We would like to know your thoughts to make Sigrid better.\n" - md += "Your username will remain confidential throughout the process.\n\n" - md += f"- ✅ [Yes, these findings are useful]({self.getFeedbackLink(options, 'useful')})\n" - md += f"- 🔸 [The findings are false positives]({self.getFeedbackLink(options, 'falsepositive')})\n" - md += f"- 🔹 [These findings are not so important to me]({self.getFeedbackLink(options, 'unimportant')})\n" + if options.feedbackURL: + md += "\n----\n\n" + md += "## Did you find this feedback helpful?\n\n" + md += "We would like to know your thoughts to make Sigrid better.\n" + md += "Your username will remain confidential throughout the process.\n\n" + md += f"- ✅ [Yes, these findings are useful]({self.getFeedbackLink(options, 'useful')})\n" + md += f"- 🔸 [The findings are false positives]({self.getFeedbackLink(options, 'falsepositive')})\n" + md += f"- 🔹 [These findings are not so important to me]({self.getFeedbackLink(options, 'unimportant')})\n" + + if self.isHtmlMarkdownSupported(): + md += "
\n" + + md += "\n----\n" + md += f"[**View this system in Sigrid**]({sigridLink})" return md def renderSummary(self, feedback, options): return f"**{self.getSummaryText(feedback, options)}**" - def renderRefactoringCandidates(self, feedback, options): + def renderRefactoringCandidates(self, feedback, sigridLink): good = self.filterRefactoringCandidates(feedback, ["improved"]) bad = self.filterRefactoringCandidates(feedback, ["introduced", "worsened"]) unchanged = self.filterRefactoringCandidates(feedback, ["unchanged"]) md = "" md += "## 👍 What went well?\n\n" - md += f"You fixed or improved **{len(good)}** refactoring candidates.\n\n" + md += f"> You fixed or improved **{len(good)}** refactoring candidates.\n\n" md += self.renderRefactoringCandidatesTable(good) + "\n" md += "## 👎 What could be better?\n\n" if len(bad) > 0: - md += f"Unfortunately, **{len(bad)}** refactoring candidates were introduced or got worse.\n\n" + md += f"> Unfortunately, **{len(bad)}** refactoring candidates were introduced or got worse.\n\n" md += self.renderRefactoringCandidatesTable(bad) + "\n" else: - md += "You did not introduce any technical debt during your changes, great job!\n\n" + md += "> You did not introduce any technical debt during your changes, great job!\n\n" md += "## 📚 Remaining technical debt\n\n" - md += f"**{len(unchanged)}** refactoring candidates didn't get better or worse, but are still present in the code you touched.\n\n" - md += self.renderRefactoringCandidatesTable(unchanged) + "\n" + md += f"> **{len(unchanged)}** refactoring candidates didn't get better or worse, but are still present in the code you touched.\n\n" + md += f"[View this system in Sigrid** to explore your technical debt]({sigridLink})\n\n" return md def renderRatingsTable(self, feedback): @@ -102,25 +113,43 @@ def renderRatingsTable(self, feedback): def filterRefactoringCandidates(self, feedback, categories): return [rc for rc in feedback["refactoringCandidates"] if rc["category"] in categories] - def sortRefactoringCandidates(self, rc): - return list(self.RISK_CATEGORY_SYMBOLS).index(rc["riskCategory"]) - def renderRefactoringCandidatesTable(self, refactoringCandidates): if len(refactoringCandidates) == 0: return "" + sortFunction = lambda rc: list(self.RISK_CATEGORY_SYMBOLS).index(rc["riskCategory"]) + sortedRefactoringCandidates = sorted(refactoringCandidates, key=sortFunction) + md = "" md += "| Risk | System property | Location |\n" md += "|------|-----------------|----------|\n" - for rc in sorted(refactoringCandidates, key=self.sortRefactoringCandidates): + for rc in sortedRefactoringCandidates[0:self.MAX_SHOWN_FINDINGS]: symbol = self.RISK_CATEGORY_SYMBOLS[rc["riskCategory"]] metricName = self.formatMetricName(rc["metric"]) metricInfo = f"**{metricName}**
({rc['category'].title()})" - location = html.escape(rc["subject"]).replace("::", "
").replace("\n", "
") + location = self.formatRefactoringCandidateLocation(rc) md += f"| {symbol} | {metricInfo} | {location} |\n" + if len(sortedRefactoringCandidates) > self.MAX_SHOWN_FINDINGS: + md += f"| ⚫️ | | + {len(sortedRefactoringCandidates) - self.MAX_SHOWN_FINDINGS} more |" + return md + "\n" + def formatRefactoringCandidateLocation(self, rc): + location = rc["subject"] + + if rc.get("occurrences") and len(rc["occurrences"]) > self.MAX_OCCURRENCES: + formatOccurrence = lambda occ: f"{occ['filePath']} (line {occ['startLine']}-{occ['endLine']})" + occurrences = [formatOccurrence(occ) for occ in rc["occurrences"][0:self.MAX_OCCURRENCES]] + location = "\n".join(occurrences) + f"\n+ {len(rc['occurrences']) - self.MAX_OCCURRENCES} occurrences" + + return html.escape(location).replace("::", "
").replace("\n", "
") + def getFeedbackLink(self, options, feedback): return f"{options.feedbackURL}?feature=sigridci.feedback&feedback={feedback}&system={options.getSystemId()}" + + def isHtmlMarkdownSupported(self): + if not self.ALLOW_FANCY_MARKDOWN: + return False + return Platform.isGitHub() or Platform.isGitLab() or Platform.isAzureDevOps() diff --git a/sigridci/sigridci/platform.py b/sigridci/sigridci/platform.py new file mode 100644 index 00000000..fe802693 --- /dev/null +++ b/sigridci/sigridci/platform.py @@ -0,0 +1,33 @@ +# Copyright Software Improvement Group +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + + +class Platform: + @staticmethod + def isGitHub(): + return "GITHUB_REPOSITORY" in os.environ + + @staticmethod + def isGitLab(): + return "GITLAB_CI" in os.environ + + @staticmethod + def isAzureDevOps(): + return "BUILD_REPOSITORY_NAME" in os.environ + + @staticmethod + def isBitBucket(): + return "BITBUCKET_REPO_SLUG" in os.environ diff --git a/test/test_markdown_report.py b/test/test_markdown_report.py index 895190fd..6a8a35d2 100644 --- a/test/test_markdown_report.py +++ b/test/test_markdown_report.py @@ -13,8 +13,9 @@ # limitations under the License. import inspect +import os import tempfile -from unittest import TestCase +from unittest import TestCase, mock from sigridci.sigridci.markdown_report import MarkdownReport from sigridci.sigridci.publish_options import PublishOptions, RunMode @@ -44,10 +45,11 @@ def testMarkdown(self): } report = MarkdownReport() + report.ALLOW_FANCY_MARKDOWN = False markdown = report.renderMarkdown("1234", feedback, self.options) expected = """ - # Sigrid maintainability feedback + # [Sigrid](https://sigrid-says.com/aap/noot) maintainability feedback **↗️ You improved your code's maintainability towards your Sigrid objective of 3.5 stars** @@ -55,12 +57,12 @@ def testMarkdown(self): ## 👍 What went well? - You fixed or improved **0** refactoring candidates. + > You fixed or improved **0** refactoring candidates. ## 👎 What could be better? - Unfortunately, **2** refactoring candidates were introduced or got worse. + > Unfortunately, **2** refactoring candidates were introduced or got worse. | Risk | System property | Location | |------|-----------------|----------| @@ -70,14 +72,11 @@ def testMarkdown(self): ## 📚 Remaining technical debt - **1** refactoring candidates didn't get better or worse, but are still present in the code you touched. + > **1** refactoring candidates didn't get better or worse, but are still present in the code you touched. - | Risk | System property | Location | - |------|-----------------|----------| - | 🔴 | **Unit Complexity**
(Unchanged) | mies | + [View this system in Sigrid** to explore your technical debt](https://sigrid-says.com/aap/noot) - - ## Sigrid ratings + ## ⭐️ Sigrid ratings | System property | System on 2022-01-10 | Before changes | New/changed code | |-----------------|-------------------------------------------|----------------|------------------| @@ -92,7 +91,6 @@ def testMarkdown(self): | **Maintainability** | **4.0** | **2.6** | **3.0** | ---- - [**View this system in Sigrid**](https://sigrid-says.com/aap/noot) """ @@ -145,6 +143,43 @@ def testSortRefactoringCandidatesTableBySeverity(self): self.assertEqual(table.strip(), inspect.cleandoc(expected).strip()) + def testLimitRefactoringCandidatesTableWhenThereAreTooMany(self): + findings = [self.toRefactoringCandidate(f"aap-{i}", "introduced", "UNIT_SIZE", "HIGH") for i in range(1, 100)] + + report = MarkdownReport() + table = report.renderRefactoringCandidatesTable(findings) + + expected = """ + | Risk | System property | Location | + |------|-----------------|----------| + | 🟠 | **Unit Size**
(Introduced) | aap-1 | + | 🟠 | **Unit Size**
(Introduced) | aap-2 | + | 🟠 | **Unit Size**
(Introduced) | aap-3 | + | 🟠 | **Unit Size**
(Introduced) | aap-4 | + | 🟠 | **Unit Size**
(Introduced) | aap-5 | + | 🟠 | **Unit Size**
(Introduced) | aap-6 | + | 🟠 | **Unit Size**
(Introduced) | aap-7 | + | 🟠 | **Unit Size**
(Introduced) | aap-8 | + | ⚫️ | | + 91 more | + """ + + self.assertEqual(table.strip(), inspect.cleandoc(expected).strip()) + + def testLimitDuplicatesWithTooManyOccurrences(self): + rc = self.toRefactoringCandidate(f"aap", "introduced", "DUPLICATION", "VERY_HIGH") + rc["occurrences"] = [self.toOccurrence(f"aap-{i}", i, i) for i in range(1, 10)] + + report = MarkdownReport() + table = report.renderRefactoringCandidatesTable([rc]) + + expected = """ + | Risk | System property | Location | + |------|-----------------|----------| + | 🔴 | **Duplication**
(Introduced) | aap-1 (line 1-1)
aap-2 (line 2-2)
aap-3 (line 3-3)
+ 6 occurrences | + """ + + self.assertEqual(table.strip(), inspect.cleandoc(expected).strip()) + def testNoRefactoringCanidatesTableWhenNoRefactoringCandidates(self): report = MarkdownReport() table = report.renderRefactoringCandidatesTable([]) @@ -252,10 +287,11 @@ def testWordsOfEncouragementWhenNoBadRefactoringCandidates(self): } report = MarkdownReport() + report.ALLOW_FANCY_MARKDOWN = False markdown = report.renderMarkdown("1234", feedback, self.options) expected = """ - # Sigrid maintainability feedback + # [Sigrid](https://sigrid-says.com/aap/noot) maintainability feedback **✅ You wrote maintainable code and achieved your Sigrid objective of 3.5 stars** @@ -263,7 +299,7 @@ def testWordsOfEncouragementWhenNoBadRefactoringCandidates(self): ## 👍 What went well? - You fixed or improved **1** refactoring candidates. + > You fixed or improved **1** refactoring candidates. | Risk | System property | Location | |------|-----------------|----------| @@ -272,18 +308,15 @@ def testWordsOfEncouragementWhenNoBadRefactoringCandidates(self): ## 👎 What could be better? - You did not introduce any technical debt during your changes, great job! + > You did not introduce any technical debt during your changes, great job! ## 📚 Remaining technical debt - **1** refactoring candidates didn't get better or worse, but are still present in the code you touched. - - | Risk | System property | Location | - |------|-----------------|----------| - | 🔴 | **Unit Complexity**
(Unchanged) | mies | + > **1** refactoring candidates didn't get better or worse, but are still present in the code you touched. + [View this system in Sigrid** to explore your technical debt](https://sigrid-says.com/aap/noot) - ## Sigrid ratings + ## ⭐️ Sigrid ratings | System property | System on 2022-01-10 | Before changes | New/changed code | |-----------------|-------------------------------------------|----------------|------------------| @@ -298,7 +331,6 @@ def testWordsOfEncouragementWhenNoBadRefactoringCandidates(self): | **Maintainability** | **3.0** | **3.1** | **4.0** | ---- - [**View this system in Sigrid**](https://sigrid-says.com/aap/noot) """ @@ -316,18 +348,16 @@ def testKeepMarkdownSimpleIfThereAreNoCodeChanges(self): } report = MarkdownReport() + report.ALLOW_FANCY_MARKDOWN = False markdown = report.renderMarkdown("1234", feedback, self.options) expected = """ - # Sigrid maintainability feedback + # [Sigrid](https://sigrid-says.com/aap/noot) maintainability feedback **💭️ You did not change any files that are measured by Sigrid** - - Sigrid compared your code against the baseline of 2022-01-10. - + ---- - [**View this system in Sigrid**](https://sigrid-says.com/aap/noot) """ @@ -353,15 +383,12 @@ def testDoNotShowAnyRefactoringCandidatesIfTheBaselineIsUnknown(self): markdown = report.renderMarkdown("1234", feedback, self.options) expected = """ - # Sigrid maintainability feedback + # [Sigrid](https://sigrid-says.com/aap/noot) maintainability feedback **💭️ You did not change any files that are measured by Sigrid** - - Sigrid compared your code against the baseline of 2022-01-10. - + ---- - [**View this system in Sigrid**](https://sigrid-says.com/aap/noot) """ @@ -379,10 +406,11 @@ def testIncludeFeedbackLinks(self): } report = MarkdownReport() + report.ALLOW_FANCY_MARKDOWN = False markdown = report.renderMarkdown("1234", feedback, self.options) expected = """ - # Sigrid maintainability feedback + # [Sigrid](https://sigrid-says.com/aap/noot) maintainability feedback **⚠️ Your code did not improve towards your Sigrid objective of 3.5 stars** @@ -390,19 +418,20 @@ def testIncludeFeedbackLinks(self): ## 👍 What went well? - You fixed or improved **0** refactoring candidates. + > You fixed or improved **0** refactoring candidates. ## 👎 What could be better? - You did not introduce any technical debt during your changes, great job! + > You did not introduce any technical debt during your changes, great job! ## 📚 Remaining technical debt - **0** refactoring candidates didn't get better or worse, but are still present in the code you touched. + > **0** refactoring candidates didn't get better or worse, but are still present in the code you touched. + [View this system in Sigrid** to explore your technical debt](https://sigrid-says.com/aap/noot) - ## Sigrid ratings + ## ⭐️ Sigrid ratings | System property | System on N/A | Before changes | New/changed code | |-----------------|-------------------------------------------|----------------|------------------| @@ -418,7 +447,74 @@ def testIncludeFeedbackLinks(self): ---- + ## Did you find this feedback helpful? + + We would like to know your thoughts to make Sigrid better. + Your username will remain confidential throughout the process. + + - ✅ [Yes, these findings are useful](https://example.com?feature=sigridci.feedback&feedback=useful&system=sig-aap-noot) + - 🔸 [The findings are false positives](https://example.com?feature=sigridci.feedback&feedback=falsepositive&system=sig-aap-noot) + - 🔹 [These findings are not so important to me](https://example.com?feature=sigridci.feedback&feedback=unimportant&system=sig-aap-noot) + + ---- [**View this system in Sigrid**](https://sigrid-says.com/aap/noot) + """ + + self.assertEqual(markdown.strip(), inspect.cleandoc(expected).strip()) + + @mock.patch.dict(os.environ, {"GITLAB_CI" : "aap/noot"}) + def testUseHtmlMarkdownOnSupportedPlatforms(self): + self.options.feedbackURL = "https://example.com" + + feedback = { + "baselineRatings": {"MAINTAINABILITY": 3.0}, + "changedCodeBeforeRatings" : {"MAINTAINABILITY" : 2.9}, + "newCodeRatings": {"MAINTAINABILITY": 2.8}, + "overallRatings": {"MAINTAINABILITY": 3.0}, + "refactoringCandidates": [] + } + + report = MarkdownReport() + markdown = report.renderMarkdown("1234", feedback, self.options) + report.ALLOW_FANCY_MARKDOWN = True + + expected = """ + # [Sigrid](https://sigrid-says.com/aap/noot) maintainability feedback + + **⚠️ Your code did not improve towards your Sigrid objective of 3.5 stars** + +
Show details + + Sigrid compared your code against the baseline of N/A. + + ## 👍 What went well? + + > You fixed or improved **0** refactoring candidates. + + + ## 👎 What could be better? + + > You did not introduce any technical debt during your changes, great job! + + ## 📚 Remaining technical debt + + > **0** refactoring candidates didn't get better or worse, but are still present in the code you touched. + + [View this system in Sigrid** to explore your technical debt](https://sigrid-says.com/aap/noot) + + ## ⭐️ Sigrid ratings + + | System property | System on N/A | Before changes | New/changed code | + |-----------------|-------------------------------------------|----------------|------------------| + | Volume | N/A | N/A | N/A | + | Duplication | N/A | N/A | N/A | + | Unit Size | N/A | N/A | N/A | + | Unit Complexity | N/A | N/A | N/A | + | Unit Interfacing | N/A | N/A | N/A | + | Module Coupling | N/A | N/A | N/A | + | Component Independence | N/A | N/A | N/A | + | Component Entanglement | N/A | N/A | N/A | + | **Maintainability** | **3.0** | **2.9** | **2.8** | ---- @@ -430,8 +526,13 @@ def testIncludeFeedbackLinks(self): - ✅ [Yes, these findings are useful](https://example.com?feature=sigridci.feedback&feedback=useful&system=sig-aap-noot) - 🔸 [The findings are false positives](https://example.com?feature=sigridci.feedback&feedback=falsepositive&system=sig-aap-noot) - 🔹 [These findings are not so important to me](https://example.com?feature=sigridci.feedback&feedback=unimportant&system=sig-aap-noot) +
+ + ---- + [**View this system in Sigrid**](https://sigrid-says.com/aap/noot) """ + self.assertTrue(report.isHtmlMarkdownSupported()) self.assertEqual(markdown.strip(), inspect.cleandoc(expected).strip()) def testDoNotIncludeFeedbackLinksIfNothingHappened(self): @@ -447,15 +548,12 @@ def testDoNotIncludeFeedbackLinksIfNothingHappened(self): markdown = report.renderMarkdown("1234", feedback, self.options) expected = """ - # Sigrid maintainability feedback + # [Sigrid](https://sigrid-says.com/aap/noot) maintainability feedback **💭️ You did not change any files that are measured by Sigrid** - - Sigrid compared your code against the baseline of N/A. - + ---- - [**View this system in Sigrid**](https://sigrid-says.com/aap/noot) """ @@ -468,3 +566,10 @@ def toRefactoringCandidate(self, subject, category, metric, riskCategory): "metric" : metric, "riskCategory" : riskCategory } + + def toOccurrence(self, file, startLine, endLine): + return { + "filePath" : file, + "startLine" : startLine, + "endLine" : endLine + }