Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Improve performance of isCommitPushed #2246

Merged
merged 3 commits into from Aug 8, 2019
Merged

Conversation

smashwilson
Copy link
Contributor

Please be sure to read the contributor's guide to the GitHub package before submitting any pull requests.

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • Suggestion: You can use checklists to keep track of progress for the sections on metrics, tests, documentation, and user research.

Description of the Change

Rework Present#isCommitPushed(sha) to only consider the push upstream of the current branch when listing remote refs that contain sha. This brings a considerable performance boost when opening CommitDetailItems in repositories that have a large number of remote refs, without actually changing any behavior because we were checking for the upstream ref in the --contains output anyway.

cc @vanessayuenn who noticed this 馃憖

Screenshot/Gif

N/A

Alternate Designs

N/A

Benefits

Performance gains when opening a CommitDetailItem in large repositories.

Possible Drawbacks

isCommitPushed will still return false if you open a commit that's pushed to a different remote tracking branch. This could cause surprising behavior if you use the github:open-commit action from the command palette to look at a commit from a different branch - specifically, the dotcom link won't show up even if the commit is pushed.

Applicable Issues

N/A

Metrics

N/A

Tests

I tested this out in dev mode with github/github.

Documentation

N/A

Release Notes

  • Fixed a performance issue when opening commit items in repositories with large numbers of remote refs.

User Experience Research (Optional)

N/A

@smashwilson smashwilson added this to In progress in Release : v0.31.0 Aug 8, 2019
@codecov
Copy link

codecov bot commented Aug 8, 2019

Codecov Report

Merging #2246 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2246      +/-   ##
==========================================
- Coverage   92.96%   92.95%   -0.01%     
==========================================
  Files         219      219              
  Lines       12565    12570       +5     
  Branches     1796     1798       +2     
==========================================
+ Hits        11681    11685       +4     
- Misses        884      885       +1
Impacted Files Coverage 螖
lib/models/repository-states/present.js 100% <100%> (酶) 猬嗭笍
lib/git-shell-out-strategy.js 99.8% <100%> (酶) 猬嗭笍
lib/atom/gutter.js 90.47% <0%> (-2.39%) 猬囷笍

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update eaf0e4a...c30bf6a. Read the comment docs.

@smashwilson smashwilson merged commit b3fd9f4 into master Aug 8, 2019
Release : v0.31.0 automation moved this from In progress to Merged Aug 8, 2019
@smashwilson smashwilson deleted the aw/is-commit-pushed-stall branch August 8, 2019 18:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

1 participant