Display commit author in preview
Created by: eseliger
Will rework the design, once we have the final version. This, in the meantime, should work, so I think it's fine to review.
Works on https://github.com/sourcegraph/sourcegraph/issues/13645
Merge request reports
Activity
Created by: codecov[bot]
Codecov Report
Merging #14181 into main will increase coverage by
0.45%
. The diff coverage is57.44%
.@@ Coverage Diff @@ ## main #14181 +/- ## ========================================== + Coverage 51.28% 51.73% +0.45% ========================================== Files 776 1536 +760 Lines 23075 77760 +54685 Branches 7072 7041 -31 ========================================== + Hits 11834 40233 +28399 - Misses 11184 33906 +22722 - Partials 57 3621 +3564
Flag Coverage Δ #go 51.92% <45.16%> (?)
#integration 30.31% <81.25%> (+0.04%)
#storybook 18.06% <25.00%> (+<0.01%)
#typescript 51.30% <81.25%> (+0.01%)
#unit 33.90% <0.00%> (-0.03%)
Impacted Files Coverage Δ cmd/frontend/graphqlbackend/campaigns.go 0.00% <ø> (ø)
cmd/frontend/graphqlbackend/git_commit.go 22.72% <0.00%> (ø)
cmd/frontend/graphqlbackend/hunk.go 0.00% <0.00%> (ø)
cmd/frontend/graphqlbackend/person.go 0.00% <0.00%> (ø)
.../frontend/graphqlbackend/repository_contributor.go 0.00% <0.00%> (ø)
...c/enterprise/campaigns/apply/CampaignApplyPage.tsx 90.90% <ø> (ø)
...c/enterprise/campaigns/apply/ChangesetSpecList.tsx 100.00% <ø> (ø)
...c/enterprise/campaigns/apply/ChangesetSpecNode.tsx 100.00% <ø> (ø)
web/src/enterprise/campaigns/apply/backend.ts 74.28% <ø> (ø)
...paigns/apply/GitBranchChangesetDescriptionInfo.tsx 76.92% <76.92%> (ø)
... and 763 more Created by: eseliger
That being said: I'm not sure whether it isn't too repetitive to show the same commit message everywhere. We already have that for the pull request titles and that I don't like 100% either
I think that is actually fine. It is in the collapsible menu fo a changeset, so the user explicitly asks for further information. For each changeset opened, I would probably like to be able to confirm the impact it has, so showing the (currently just) repeated data feels not too weird to me. Happy to take suggestions though. Also, please note this design is not final :) I just wanted to close the main work here, so I can focus on other things first and do the quick design overhaul once we have the designs.
And, that being said part 2: with the commit message it might be possible to simplify the design of the preview a bit and get rid of the headers. If we show the commit message we probably don't need to show "Commits" and "Diff" as headers, since the context makes it pretty clear what is what. Maybe we can take some inspiration from GitHub:
< image >
Hmm, I think why GitHub doesn't need it there is because you purposefully went on a page to view a commit there, hence the entity expected is the commit. With out list of changesets, though, I think especially for first-time-users it's not immediately clear what additional information you can get on a changeset, and having those two short guidelines helps grasping that you can get the commits created and the overall diff here in this view. Not sure that makes sense, but that was how I felt about it. Additionally, I'll also talk about this with design and it was just my personal note added here, until design's ready.