go-github icon indicating copy to clipboard operation
go-github copied to clipboard

Remove redundant fields from `Environment` struct

Open jvm986 opened this issue 2 years ago • 1 comments

The Environment struct contains fields that do not exists in the response. As per the docs, and in practice -->

curl -s -H "Authorization: token <REDACTED> " https://api.github.com/repos/jvm986/test/environments/TEST

⬇️

{
  "id": 953594922,
  "node_id": "EN_kwDOJR_BYs441rQq",
  "name": "TEST",
  "url": "https://api.github.com/repos/jvm986/test/environments/TEST",
  "html_url": "https://github.com/jvm986/test/deployments/activity_log?environments_filter=TEST",
  "created_at": "2023-04-03T07:03:05Z",
  "updated_at": "2023-04-03T07:03:05Z",
  "can_admins_bypass": false,
  "protection_rules": [
    {
      "id": 7039700,
      "node_id": "GA_kwDOJR_BYs4Aa2rU",
      "type": "branch_policy"
    }
  ],
  "deployment_branch_policy": {
    "protected_branches": true,
    "custom_branch_policies": false
  }
}

Remove the fields that are not found in the response , specifically -->

Owner                  *string         `json:"owner,omitempty"`
Repo                   *string         `json:"repo,omitempty"`
EnvironmentName        *string         `json:"environment_name,omitempty"`
WaitTimer              *int            `json:"wait_timer,omitempty"`
Reviewers              []*EnvReviewers `json:"reviewers,omitempty"`
DeploymentBranchPolicy *BranchPolicy   `json:"deployment_branch_policy,omitempty"`

While this is technically a breaking change, as I understand it, anyone using these fields will be getting nil data in them.

NB. As I am sure there is a reason these fields were included in the first place, I would love to hear it!

jvm986 avatar Apr 03 '23 07:04 jvm986

NB. As I am sure there is a reason these fields were included in the first place, I would love to hear it!

Yes, there are three reasons why fields have been added in this repo:

  • they are in the official docs,
  • they are discovered during use,
  • the structs are used by multiple endpoints

So it is much more challenging to remove fields from this repo and we certainly cannot do so based on the return values found from a single endpoint.

To do this properly, we should at the very least read through every PR where each field was added (using GitHub's "blame" feature) and possibly even contact GitHub tech support unless the official documentation states that a field was removed, which does indeed sometimes happen.

Therefore, we will not be moving quickly with this PR until we have a better understanding of each affected field.

gmlewis avatar Apr 03 '23 11:04 gmlewis