Skip to content
Snippets Groups Projects

Display commit author in preview

Merged Administrator requested to merge es/changeset-author-display into main

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

Merged by avatar (Jul 25, 2025 11:35pm UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Created by: codecov[bot]

    Codecov Report

    Merging #14181 into main will increase coverage by 0.45%. The diff coverage is 57.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%) :arrow_up:
    #storybook 18.06% <25.00%> (+<0.01%) :arrow_up:
    #typescript 51.30% <81.25%> (+0.01%) :arrow_up:
    #unit 33.90% <0.00%> (-0.03%) :arrow_down:
    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 :grin:

    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.

  • Created by: eseliger

    Addressed feedback, design is simplified as per a quick design session yesterday, so merging for now so we have the necessary foundation of this feature merged.

Please register or sign in to reply
Loading