-
Notifications
You must be signed in to change notification settings - Fork 152
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.
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
Set default branch from "master" to "HEAD" #510
Conversation
README.md
Outdated
[![Windows Build status](https://ci.appveyor.com/api/projects/status/github/r-lib/remotes?svg=true)](https://ci.appveyor.com/project/gaborcsardi/remotes) | ||
[![](https://www.r-pkg.org/badges/version/remotes)](https://www.r-pkg.org/pkg/remotes) | ||
[![CRAN RStudio mirror downloads](https://cranlogs.r-pkg.org/badges/remotes)](https://www.r-pkg.org/pkg/remotes) | ||
[![Coverage Status](https://img.shields.io/codecov/c/github/r-lib/remotes/master.svg)](https://codecov.io/github/r-lib/remotes?branch=master) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to keep ?branch=xyz
here and for the travis case, otherwise the badge would show the status of the last running branch instead of the default one.
While I agree that a change from 'master' as the default branch of remotes would be preferable, I think we should wait to change the default branches on this and other repos until GitHub or git officially changes to a different default branch name. Right now the only 'official' word on this from GitHub is a twitter reply (https://twitter.com/natfriedman/status/1271253144442253312), and this change will likely break a number of workflows, so it needs to be done carefully. For now could you update this PR to keep the default branch for remotes as 'master', and add a bullet to NEWS? It should briefly describe the change and end with |
I understand the apprehension, as discussed on the Issue, some people's workflows will be largely affected. However, I believe it useful to have this ready to go and at some point this change will be needed. I've added to the NEWS.md file as requested. This is actually my first PR (other than in my own repos), so any feedback on my style, etc... would be greatly appreciated. |
I've addressed the comments by @gaborcsardi and @jimhester in my local clone of I currently have two errors being hit by the
There is also a warning being flagged when I run
I have removed a reference to |
Currently the only reason for the build failures is due to an issue with postmanlabs/httpbin#617. |
This PR replaces
"master"
within reference to repo branches with"HEAD"
, which grabs the latest commit from thedefault
branch. This allows for repos that do not use the"master"
convention to be targeted for download without the need to specify a reference (e.g."main"
).This will not effect repos that already use
"master"
as their default branch, as it will still be pulled. The only situation negatively effected by this PR is when a user is trying to target the"master"
branch (and is thus using the previous default,ref="master"
) and it is not the default, which will be a rare edgecase.It is also suggested that the
remotes
repository changes it's default branch to"main"
. Currently codecov requires a named branch and is incompatible with"HEAD"
, and so the badge (as it stands in this PR) will not display until this is done.This resolves Issue #508