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.

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 defaults for the max length of PR branch names #7564

Open
jeffwidman opened this issue Jul 15, 2023 · 1 comment
Open

Set defaults for the max length of PR branch names #7564

jeffwidman opened this issue Jul 15, 2023 · 1 comment
Labels
T: feature-request Requests for new features V: AWS CodeCommit Issues related to AWS CodeCommit support V: Azure Devops Issues relating to Azure Devops support V: Bitbucket Issues relating to Bitbucket support V: GitLab Issues relating to GitLab support

Comments

@jeffwidman
Copy link
Member

jeffwidman commented Jul 15, 2023

This PR added a param for setting max length of the branch name:

However, we don't set a default value, which means many of our users never benefit from this. In fact, several months later we were investigating errors for :dependabot: hosted on on GitHub.com and realized we ourselves had forgotten to set this value!

So I vote we start setting a reasonable default for each platform within dependabot-core.

This is similar to what I suggested in:

Implementing platform-defaults at first glance seems like a backwards-breaking change. However, given that branch names longer than these limits would already be erroring/failing to create, I don't think there's any backwards compatibility issue.

🤔 Do all users of this param set it to the max allowed by their platform? I can't think of a reason to pick a different value... So once we start setting that within dependabot-core, should we also remove the top-level configurable branch_name_max_length param? Might as well keep our public API surface small...

:branch_name_prefix, :branch_name_max_length, :github_redirection_service,

@TomNaessens, you've got the most context here, what say you?

cc @SimonSomlai, @nudded as appears you also wanted this PR, do you simply set this value to the max of your platform?

@jeffwidman jeffwidman added T: feature-request Requests for new features V: GitLab Issues relating to GitLab support V: Bitbucket Issues relating to Bitbucket support V: Azure Devops Issues relating to Azure Devops support V: AWS CodeCommit Issues related to AWS CodeCommit support labels Jul 15, 2023
@jeffwidman jeffwidman changed the title Set reasonable defaults for the max length of PR branch names Set internal defaults for the max length of PR branch names Jul 15, 2023
@TomNaessens
Copy link

TomNaessens commented Jul 15, 2023

I can mostly only comment from a Gitlab (through dependabot-gitlab) point of view as I haven't used the max length setting outside of Gitlab.

To answer your question first:

Do all users of this param set it to the max allowed by their platform? I can't think of a reason to pick a different value...

No, users do set it lower than the max value for the platform. For example, on the Gitlab codebase I'm most active on, we have specific branch limits (53 chars) that are enforced by us (so not by Gitlab) as our staging infrastructure uses those branch names, and Kubernetes has specific branch name limits.
So, dropping the removing top-level configurable branch_name_max_length param would break the dependabot-gitlab part that allows us to set a specific max length for our use case. Relevant pieces of code in dependabot-gitlab are:

About the general proposal:

I vote we start setting a reasonable default for each platform within dependabot-core

That sounds reasonable to me! 😄 Like you already mentioned, it's a change that users would be able to benefit from "out of the box".

Only small thing I can think of is platforms having different limits for different versions of their platform. I can imagine that newer versions might enlarge the maximum value of fields, and when this is reflected in a dependabot-core default change, that might break it for self hosted instances of those platforms that have not been updated to that version (but run the latest version of dependabot-core). But that can then be solved again by explicitly limiting the max length in their configs, so I don't think that should be accounted for.

@jeffwidman jeffwidman changed the title Set internal defaults for the max length of PR branch names Set defaults for the max length of PR branch names Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: feature-request Requests for new features V: AWS CodeCommit Issues related to AWS CodeCommit support V: Azure Devops Issues relating to Azure Devops support V: Bitbucket Issues relating to Bitbucket support V: GitLab Issues relating to GitLab support
Projects
None yet
Development

No branches or pull requests

2 participants