danger / swift

⚠️ Stop saying "you forgot to …" in code review
https://danger.systems/swift/
MIT License
1.05k stars 140 forks source link

Danger-swift should default to `main` and not `master` #613

Open ls-philippe-casgrain opened 3 months ago

ls-philippe-casgrain commented 3 months ago

It's been almost four years since Github decided to rename master to main, and we still have to specify --base main in Danger lest we get an error.

I'm not talking about renaming this repository's master branch, that's up to you. But having a sensible default when using Danger on other repos seems like a logical thing to do.

f-meloni commented 3 months ago

I don't have any strong opinion on whether main or master should be used, however I do expect that someone could make the opposite argument to have master as default if their team is using master instead of main (for example because historically there have been many repos already created that used master instead of main). While I agree that given GitHub changed its default it is sensible to assume that what you propose is the best decision long term I also want to suggest an additional thing (not as replacement of what you suggest but as additional potential feature for the tool): We could have something optional on the repo that danger can read (e.g., a config.danger file, or an environment variable - my preference is probably for the latter) where the branch name is specified, so that no one needs to keep adding --base every time in their main danger workflow.

ls-philippe-casgrain commented 3 months ago

It's trivial to rename a repo from master to main, so I suspect that the number of repos still named master is dwindling. As such, it might be time to perform the switch in Danger. You are, of course, free to do what you want, but in my experience adding yet another configuration option often adds complexity for very little gain. Danger is already of the opinion that the default should be master. All I'm saying is the opinion should be changed.

417-72KI commented 1 month ago

I think this is an issue for danger-js, instead of danger-swift.

There are come codes that master is defined as a default branch. e.g. https://github.com/danger/danger-js/blob/b67d710af4d177d57e7091a742da54a2e9fa37fc/source/platforms/LocalGit.ts#L28