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

Make Sigrid CI feedback more visually appealing. #530

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/reference/release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
79 changes: 54 additions & 25 deletions sigridci/sigridci/markdown_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -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" : "🔴",
Expand All @@ -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 += "<details><summary>Show details</summary>\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 += "</details>\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):
Expand All @@ -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}**<br />({rc['category'].title()})"
location = html.escape(rc["subject"]).replace("::", "<br />").replace("\n", "<br />")
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("::", "<br />").replace("\n", "<br />")

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()
33 changes: 33 additions & 0 deletions sigridci/sigridci/platform.py
Original file line number Diff line number Diff line change
@@ -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 "CI_PROJECT_PATH" in os.environ

@staticmethod
def isAzureDevOps():
return "BUILD_REPOSITORY_NAME" in os.environ

@staticmethod
def isBitBucket():
return "BITBUCKET_REPO_SLUG" in os.environ
Loading