Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Link to current branch in More's Compare and Commits links #2102

Merged
merged 8 commits into from Jun 4, 2019
Merged

Link to current branch in More's Compare and Commits links #2102

merged 8 commits into from Jun 4, 2019

Conversation

jerone
Copy link
Contributor

@jerone jerone commented May 30, 2019 •
edited

Description

Include current ref (branch or tag) in compare & commit repo links added in more-dropdown. Different pages required different detection to get the current ref (branch or tag).

Extra

Optionally I can include the default branch, but that requires an extra API call and is not necessary.

Closes

Closes #1197.

Test

  1. Visit any of the below urls.
  2. Open repo tab "More" -> "Compare" or "Commits".
  3. See it link to the correct tree (branch or tag).

fregante
fregante requested changes May 30, 2019
View reviewed changes
Copy link
Member

@fregante fregante left a comment •
edited

Choose a reason for hiding this comment

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

🦋

source/features/more-dropdown.tsx Outdated Show resolved Hide resolved
source/features/more-dropdown.tsx Outdated Show resolved Hide resolved
source/features/more-dropdown.tsx Outdated Show resolved Hide resolved
source/features/more-dropdown.tsx Show resolved Hide resolved
@jerone
Copy link
Contributor Author

jerone commented May 30, 2019

Processed your feedback and fixed linting.

@fregante fregante changed the title Include current branch/tag in compare & commit repo links. Closes #1197 Include current branch/tag in compare & commit repo links May 31, 2019
@jerone
Moved ref detection to utils & added tests
6f91784 
Fix compare detection.
Add blame detection.
Prioritized order.
@jerone
Detect pr commit
28676e7
@fregante
Copy link
Member

fregante commented Jun 2, 2019 •
edited

I don't think this should affect Compare pages. The only reason why I'd use that link when already on the Compare page would be to "reset the view"

Tags shouldn't be affected either, e.g. https://github.com/BrowserSync/browser-sync/releases/tag/v2.26.3, for two reasons:

  • The generated link is always empty (master...v2.26.3)
  • There's already a compare link on the page named "8 commits on master since this tag", which compares in the right direction (v2.26.3...master)

@jerone
Copy link
Contributor Author

jerone commented Jun 2, 2019

Removed compare & tags/releases detection.

@fregante
Copy link
Member

fregante commented Jun 2, 2019

I just saw isPRCommit too, that also should probably be dropped, it's too specific I think

@jerone
Copy link
Contributor Author

jerone commented Jun 2, 2019

Removed PR commit detection too.

@fregante fregante changed the title Include current branch/tag in compare & commit repo links Link to current branch in More's Compare and Commits links Jun 4, 2019
@fregante fregante merged commit 0a6b6b4 into refined-github:master Jun 4, 2019
@fregante
Copy link
Member

fregante commented Jun 4, 2019

Thanks @jerone 🍫

@jerone jerone deleted the features/jvw-#1197-branchify-compare-commits branch June 4, 2019 18:32
@jerone
Copy link
Contributor Author

jerone commented Jun 4, 2019

More LOC for tests, then for the real fix. 🎉😂

@jerone jerone mentioned this pull request Jun 4, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fregante fregante fregante approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"Compare" repo link should include the current branch/tag
2 participants
@jerone @fregante