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

Export branches attribute of github_repository resource #959

Merged

Conversation

muru
Copy link
Contributor

@muru muru commented Oct 28, 2021

In #892, the newly-added branches attribute was exported for the data source, but not the resource. This results in an error as noted in #892 (comment). This PR exports the branches attribute in the resource too.


I considered adding a test, but it looks like a branch might need to exist, and in cases where multiple branches exist, the order of the branch names might not be deterministic. So I tested this manually by using the built provider in a Terraform project using dev_overrides.

In integrations#892, the newly-added `branches` attribute was exported for the data
source, but not the resource. This exports `branches` in the resource
too.
@muru muru changed the title Export branches attribute of repository resource Export branches attribute of github_repository resource Oct 28, 2021
@tobiassjosten
Copy link
Contributor

We recently started using the GitHub provider and this missing feature is unfortunately a blocker for us. Checking the PR it looks pretty straightforward. :) Please let me know if there's anything we can do to help get this merged and released! Would love to contribute if we can.

Is it @jcudit or perhaps @tarebyte who maintains this, btw?

@ranesagar
Copy link

Getting the list of branches from the resource will be really helpful. +1 on making this available.

@tarebyte
Copy link
Member

perhaps @tarebyte who maintains this, btw?

I've never seen this codebase before, so I'm not sure why my name came up 😅

@tobiassjosten
Copy link
Contributor

I've never seen this codebase before, so I'm not sure why my name came up 😅

My bad! I checked what team owned this repo and your profile popped up.

Any idea whom one could talk to about merging this in? It's a tiny, straightforward change and it'd make such a difference for us! 🙏

@tarebyte
Copy link
Member

My bad! I checked what team owned this repo and your profile popped up.

Which team is that?

Any idea whom one could talk to about merging this in? It's a tiny, straightforward change and it'd make such a difference for us! 🙏

Sorry I don't have clue, my team doesn't own anything to do with this hence my confusion.

@tobiassjosten
Copy link
Contributor

My bad! I checked what team owned this repo and your profile popped up.

Which team is that?

The GitHub integrations team. You're one of four people listed and the only one with a Staff badge. :)

@jcudit jcudit added this to the v5.0.0 milestone Nov 23, 2021
@tarebyte
Copy link
Member

CleanShot 2021-11-30 at 11 11 53@2x

Fascinating, unfortunately I don't even have permission to merge your PR 😅

@tarebyte
Copy link
Member

@integrations/integrations-reviewers is this in your AoR?

@tobiassjosten
Copy link
Contributor

@jcudit Is this considered a BC break or why the major bump? :) Maybe im missing something with how these milestones are used.

@jcudit
Copy link
Contributor

jcudit commented Nov 30, 2021

@tobiassjosten my mistake and thanks for bringing this back to my attention. Will get this linked to the next upcoming minor release!

@jcudit jcudit modified the milestones: v5.0.0, v4.19.0 Nov 30, 2021
Copy link
Contributor

@jcudit jcudit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙇🏾

@jcudit jcudit merged commit 3444b63 into integrations:main Dec 3, 2021
kfcampbell pushed a commit to kfcampbell/terraform-provider-github that referenced this pull request Jul 26, 2022
In integrations#892, the newly-added `branches` attribute was exported for the data
source, but not the resource. This exports `branches` in the resource
too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants