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

Combine Brach and Tag Options #153

Open
DataDavD opened this issue Jul 22, 2021 · 3 comments
Open

Combine Brach and Tag Options #153

DataDavD opened this issue Jul 22, 2021 · 3 comments
Assignees

Comments

@DataDavD
Copy link
Contributor

DataDavD commented Jul 22, 2021

Currently Branches and Tags have separate Options structs although they are essentially the same endpoint. RepoBranchesOptions is only different from RepoTagsOptions in that it possess a BranchName field as shown here

go-bitbucket/bitbucket.go

Lines 202 to 211 in dd20750

type RepositoryBranchOptions struct {
Owner string `json:"owner"`
RepoSlug string `json:"repo_slug"`
Query string `json:"query"`
Sort string `json:"sort"`
PageNum int `json:"page"`
Pagelen int `json:"pagelen"`
MaxDepth int `json:"max_depth"`
BranchName string `json:"branch_name"`
}

and

go-bitbucket/bitbucket.go

Lines 224 to 232 in dd20750

type RepositoryTagOptions struct {
Owner string `json:"owner"`
RepoSlug string `json:"repo_slug"`
Query string `json:"q"`
Sort string `json:"sort"`
PageNum int `json:"page"`
Pagelen int `json:"pagelen"`
MaxDepth int `json:"max_depth"`
}

I propose that we combine the repo branches and tag option structs into a single RepositoryRefOptions (branches and tags are both refs anyways) and instead of a BranchName field just have a Name field that represents either the branch name or tag name. This would allow us to have a single ListRefs function instead of two separate functions for listing branches and listing tags. That way users can use the same options struct to access a single function for listing refs (i.e. branches or tags) and it can make use of the existing GetBranch function to also get a tag.

So, @ktrysmt If this proposal is acceptable and you agree, can you assign me to do this refactor please. For context, I've already started working on this for fun in my fork.

Thanks in advance.

Best,
dd

@DataDavD
Copy link
Contributor Author

For references we could possible include another field inside the Options struct that is a boolean for determining whether or not the refoptions describes a branch or tag so that it handles easily the funcs for the GetBranch or GetTag (GetTag to be added as part of the PR to resolve this issue if you approve @ktrysmt )

@ktrysmt
Copy link
Owner

ktrysmt commented Jul 23, 2021

Exactly. These options are duplicated likely. It seems a good idea. @DataDavD

@DataDavD
Copy link
Contributor Author

Ok great I'll start on it.

Can you please assign me to this issue

DataDavD added a commit to DataDavD/go-bitbucket that referenced this issue Jul 30, 2021
This commit refactors the existing RepositoryBranchTarget and
RepositoryTagTarget types to a single RepositoryRefTarget since tags and
branches are both refs and the two Target variables represented the
same type anyways. This will also help us move to removing branch
and tag specific options types into a single refs options type.

See ktrysmt#155 and
ktrysmt#153 (comment)
DataDavD added a commit to DataDavD/go-bitbucket that referenced this issue Dec 8, 2021
This commit refactors the existing RepositoryBranchTarget and
RepositoryTagTarget types to a single RepositoryRefTarget since tags and
branches are both refs and the two Target variables represented the
same type anyways. This will also help us move to removing branch
and tag specific options types into a single refs options type.

See ktrysmt#155 and
ktrysmt#153 (comment)
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

No branches or pull requests

2 participants