packagekit: link Red Hat issue references in changelogs - #23117
packagekit: link Red Hat issue references in changelogs#23117tarunvashishth wants to merge 3 commits into
Conversation
Venefilyn
left a comment
Venefilyn
left a comment
There was a problem hiding this comment.
Thanks! In short I like it but atm I'm skeptical about how the Jira linking is supposed to work. The RHBZ can be made non-distro specific too
| if (!text || !isRedHatBasedDistribution()) | ||
| return text; | ||
|
|
||
| const issuePattern = /(rhbz|jira)#(\d+)/g; |
There was a problem hiding this comment.
I think we can check for rhbz no matter the distro tbh. I don't think anyone else refers to their trackers as rhbz and it is quite normal to link to in regards to CVEs etc.
However, with Jira I agree that we should only link to it within RHEL-sphere, but is it common to write jira#1234? That feels very strange to me as it doesn't include the namespace at all like RHEL-1234.
Do you have links to where the jira#1234 syntax is used?
There was a problem hiding this comment.
I couldn’t find solid evidence that jira#1234 is being used; most Red Hat references I found are RHEL-1234. Should I drop the Jira part and add something like RHEL-1234?
There was a problem hiding this comment.
Even if we use RHEL-1234 there are many many other projects in Jira, not to mention other Jira instances from other companies might have similar naming. Could you give the RH refs you found for RHEL-1234?
There was a problem hiding this comment.
Here are some I found:
https://redhat.atlassian.net/browse/RHEL-56139
https://redhat.atlassian.net/browse/RHEL-84657
There was a problem hiding this comment.
Ack. Then lets go with that for now.
So I'd request to make
- RHBZ linking not specific to Fedora-based distros
- Add checking for RHEL-\d+ for Fedora-based distros
At some point we change to an object with keys to a list with all the objects in it and looking at ID instead. To make it work during manual testing we need to update the syntax from object to array. Signed-off-by: Freya Gustavsson <freya@venefilyn.se>
| const [, tracker, issueId] = match; | ||
| const href = tracker === "rhbz" | ||
| ? `https://bugzilla.redhat.com/show_bug.cgi?id=${issueId}` | ||
| : `https://issues.redhat.com/browse/${issueId}`; |
There was a problem hiding this comment.
This should now be https://redhat.atlassian.net
| } else { | ||
| description = <div className="changelog">{info.description}</div>; | ||
| descriptionFirstLine = renderChangelogWithIssueLinks(descriptionFirstLine); | ||
| description = <div className="changelog">{renderChangelogWithIssueLinks(info.description)}</div>; |
There was a problem hiding this comment.
I was checking this out now to see how it would look. There is one issue that makes me think we need to modify description for entries with markdown as well since these otherwise don't get changes. Which does create a bit of a jarring experience where some has links and some don't, or right out doesn't work at all as all changelog entries are markdown for the distro (feels like it on Fedora based on the results I get)
While we can't change everything to markdown as it would break for some packages that are supposed to be plaintext, we could modify the markdown or the HTML generated from markdown as long as it doesn't match
[rhbz#1234](http...)[rhbz#1234 'aria label'](http...)<a href="http..." aria-label="aria label">rhbz#1234</a>
Probably the easiest is to detect if it's DOM in renderChangelogWithIssueLinks and in that case check that the text matched isn't directly within an anchor element. Then again matching wouldn't be that easy without converting to DOM first and searching for it I think..
@mvollmer WDYT?
3 participants
closes #17748