-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Default to branch name main
rather than master
#5617
Comments
Good point @mooeypoo, we usually show in the docs the default value, but I think we should use I think what we really want, is for the CMS to use the default branch instead of defaulting to That would require some more work though. Also we would need to do it in a non breaking way:
All of the above should happen where no branch is configured. WDYT? |
Ah! I was hoping for that as well, but I didn't want to overstep and increase your workload for a feature I wasn't entirely sure about its complexity ;) I figured an initial fix (either 'main' in defaults, or even just recommending 'main' in the comment?) would be a good step forward. That said -- I'm talking out of my usual expertise-bubble here, so I'm not actually sure, but doesn't github provide data about what is the default branch? I'm wondering if it's possible to use that instead of guessing too much (with a backwards compat condition). I might be wrong here, but if I remember correctly, deploying to netlify from github seemed to have auto-recognize my In any case this:
-- sounds good to me. I can take a look at submitting a PR for it. Thanks! |
Updating the docs is a great first improvement 💯
That is what I'm going for in step 3. in my comment. |
Ha, I misread your step 3 (probably because I was in a meeting, that'll teach me...) I've started working on a PR, but I'm running into a couple of conceptual issues here. I initially thought I'd need to create the method that fetches the default branch, but that method seems to already exist at https://github.com/netlify/netlify-cms/blob/65939d978eedda86526dbafc829f8e662442f227/packages/netlify-cms-backend-github/src/API.ts#L1184 (Added correction: I saw later that the above method isn't exactly what I need since it assumes This method is async, though, so adding it to a process that resolves the branch name transforms A second problem I ran into is that I couldn't find a way to mock the result from github -- so even calling the methods In any case, I am a little worried about the change to async and its potential implications on the rest of the code. I'm not 100% I'm not overcomplicating things here, though. Am I missing something that resolves this? Do we get general details earlier? Should I change the call but then go over all potential places that call the I can submit this as a PR but I wanted to pause and ask before I go through a much deeper change here in case my current direction might be misguided. Here's what I have so far: I changed // Temporarily assign until resolved
this.branch = config.backend.branch || 'main'; // this is for testing purposes to see what the unit tests return; it should be set to `master` eventually for backwards compatibility
this.resolveDefaultBranchName(config.backend.branch).then(result => {
this.branch = result;
}); and then added the method to resolve the default branch name: async resolveDefaultBranchName(name = '') {
if (name.trim()) {
// TODO: If we're already here, we can also validate
// that the branch name given in config exists,
// and if it doesn't, we can treat it as if it wasn't given
// so we fall back to the rest of the operation
return name;
}
if (await this.api?.getBranch('master')) {
// For backwards compatibility:
// If no branch was given but 'master' exists, fall back on it
return 'master';
}
// Get default branch, or fall back on 'main'
const def = await this.api?.getDefaultBranch();
return def?.name || 'main';
} I've added a test for |
Hi @mooeypoo, I believe you need to add an Then you should be able to resolve the branch in the |
Hello @mooeypoo . Are you still working on this issue? |
I was for a bit, and then drowned a bit at work and discovered I have much less time than I wanted... |
Hello @erezrokah . This is what you said in order to resolve branch when no branch name is specified in config:
I don't understand why step 1 and 2 are neccessary? Is it because there are already users who leave out |
💯
|
Masterfully explained 💯. Thank you. I don't have Gitlab or Bitbucket account but is it also an issue on those services? AFAIK they used to name default branch |
We'll need to verify, but using the same suggested logic like GitHub should work for GitLab and Bitbucket too, correct? |
I haven't dived into it yet, but your suggestion sounds good. Will get back with what I find soon. |
Hello @erezrokah. I've realized that I've only worked with the editor in Netlify CMS and thus never set up the app locally for debugging the backend. If I understand correctly from this "Contributing" guide, I would need to add at least backend name and a legit repo path to config, like this:
Is it correct? |
Hi @bytrangle, looks like you already figured that one out in #5814 |
Hello @erezrokah . I still don't understand how to test all the local changes to the backend libraries for Github, Gitlab, Bitbucket etc. Suppose I've done some changes to how the default branch is set for Github users. To see those changes, I'd adjust the config under
Now, I need to make those same changes to Gitlab. I need to change the config to point to a valid Gitlab repo and a live site where that repo is deployed.
After I finish testing with Gitlab, I repeat the steps above for Bitbucket and possibly Azure. Is there a better way to do that? |
Hi @bytrangle, that's the correct way to test backend changes. FYI, our automation uses pre-recorded data to test the backends, but that I'm not sure we need to update those tests for the linked PR. |
Hi, was curious on the status of this task; We are also using Decap in an enterprise that flags us for uninclusive language so having master replaced to main would be very helpful. |
The default branch has been renamed from |
Describe the bug
The documents reference a default branch
master
; this should be changed tomain
if possible, especially seeing as the Netlify deployment documents seem to encourage that too, and github's changing its default tomain
as well.To Reproduce
branch: master # Branch to update (optional; defaults to master)
Expected behavior
I'd expect the branch to default to
main
, or at least the documentation to acknowledge that's becoming the new default in most places.The text was updated successfully, but these errors were encountered: