Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

feat(Source): Properly render multi-line changelist messages from Perforce#63728

Merged
mmanela merged 4 commits into
mainfrom
mmanela-src-431-perforce-multiline-change-list-description-not-shown
Jul 9, 2024
Merged

feat(Source): Properly render multi-line changelist messages from Perforce#63728
mmanela merged 4 commits into
mainfrom
mmanela-src-431-perforce-multiline-change-list-description-not-shown

Conversation

@mmanela

@mmanela mmanela commented Jul 9, 2024

Copy link
Copy Markdown
Contributor

Fixes SRC-431

Mutli-line perforce changelist descriptions were not being handle since the code implicitly assumed they could not exist. This change enables support for them.

Screenshots

Changelist view with both single and multiline commits
image

Changelist view with expanded commit message
image

Individual changelist item
image

Test plan

  1. Update unit tests
  2. Validate both P4 and non-p4 commits work
  3. Validate on s2

Changelog

  • Properly render multi-line perforce changelist descriptions

Comment thread cmd/frontend/graphqlbackend/git_commit_test.go
Comment thread cmd/frontend/graphqlbackend/git_commit.go Outdated
Comment thread cmd/frontend/graphqlbackend/perforce_changelist.go Outdated
// For depots converted with git-p4, this special handling is NOT required.
func maybeTransformP4Subject(ctx context.Context, repoResolver *RepositoryResolver, commit *gitdomain.Commit) *string {
if repoResolver.isPerforceDepot(ctx) && strings.HasPrefix(commit.Message.Body(), "[p4-fusion") {
if repoResolver.isPerforceDepot(ctx) && strings.Contains(commit.Message.Body(), "[p4-fusion") {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this change should ideally also have failed a test 😆

@eseliger eseliger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yay! 5.5.0 here we come :)

Comment thread cmd/frontend/graphqlbackend/perforce_changelist_test.go
@mmanela mmanela merged commit 4c3985e into main Jul 9, 2024
@mmanela mmanela deleted the mmanela-src-431-perforce-multiline-change-list-description-not-shown branch July 9, 2024 21:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants