Environment custom branch policies support type ('branch', 'tag')

[goruha]

What

• Added type support for environment deployment_branch_policy custom branches.

Why

• Environment deployment_branch_policy supports custom branches type - branch or tag. The type is an option parameter that sets branch by default. These changes allow us to specify deployment_branch_policy for tag

Config example

environments:
  - name: development
    deployment_branch_policy:
      custom_branches:
        - dev/*
        - name: release/*
          type: branch
        - name: v*
          type: tag

You can specify custom_branches list item as string for back-compatibility or as object

name: `string`
type: `branch | tag`

Releated links

• Create a deployment branch policy API

[vercel]

@goruha is attempting to deploy a commit to the Repository Settings Team on Vercel.

A member of the Team first needs to authorize it.

[goruha]

@julekirk @mayliang021 @travi
may I have a review pls?

[travi]

supporting tag targets feels separate from what makes sense under "custom_branches". would a separate section for tags be a better fit?

[goruha]

@travi

supporting tag targets feels separate from what makes sense under "custom_branches". would a separate section for tags be a better fit?

Yeah, the separate settings for tags look better.
However, GitHub API does not have this separation, and we should follow the GitHub API structure.
Mapping the GitHub API contract to settings.yml solves the lack of documentation - you can always reference GitHub API to see what fields are available.
So I advocate to go with solution provided in the PR

https://docs.github.com/en/rest/deployments/branch-policies?apiVersion=2022-11-28#create-a-deployment-branch-policy

[goruha]

@travi can we have this PR merged?

[travi]

Yeah, the separate settings for tags look better.
However, GitHub API does not have this separation, and we should follow the GitHub API structure.
Mapping the GitHub API contract to settings.yml solves the lack of documentation - you can always reference GitHub API to see what fields are available.

in general, i agree. however, as mentioned in the docs for the environments plugin, we already deviate a bit, so that is the reason that gave me pause here. since even the github api still uses the branches terminology for this, i think i'm ok with proceeding with the proposed approach

[travi]

i appreciate that you tracked down how to accomplish this and i think this is totally valid for the scenario that defines type, but i think this change now results in not having a scenario tested for the option to not include type, doesnt it. it seems like we probably should define an additional scenario for including type in https://github.com/repository-settings/app/blob/master/test/integration/features/environments.feature

i also wonder if the inclusion of a type that doesnt match the default requires any additional care to avoid unnecessary update requests when the intended value matches what is in the api. could you verify that comparing an existing rule in the api avoids extra calls when the type is and is not present in the settings.yml? if that requires additional logic to avoid extra calls, we may need test scenarios for those cases as well

[goruha]

Actually, we always specify type for API calls.
https://github.com/repository-settings/app/pull/910/files#diff-d3173e671826d864f026b433955eeef4008090a5fc6c3111f8ec2e3977e2bc4dR156

So, the test is correct and does not dismiss the case without type.

[travi]

because of the default values, i think we also need to handle similar in

app/lib/plugins/environments.js

Lines 54 to 59 in 36173c5

	function deploymentBranchPolicyHasChanged (existing, attrs) {
	return (
	deploymentBranchPolicyToString(existing.deployment_branch_policy) !==
	deploymentBranchPolicyToString(attrs.deployment_branch_policy)
	)
	}

to avoid repetitively updating rules that are already in an ok state.

[goruha]

@travi, thanks for the nice catch.
I'll address this comment this week.