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

Make all option struct fields pointers #1294

Merged
merged 1 commit into from Jan 4, 2022
Merged

Conversation

svanharmelen
Copy link
Member

Fixes #427 #1292

@mitar
Copy link
Contributor

mitar commented Dec 2, 2021

If we are doing this breaking change, could we also make sure to get rid of all []*Struct and convert them into []Struct? For example:

type ReleaseAssets struct {
	Links []*ReleaseAssetLink `url:"links" json:"links"`
}

There is no point in having * there. Does API even survive sending a null inside the JSON array?

@svanharmelen
Copy link
Member Author

There is no point in having * there that depends of how and where your use the items from the slice I guess. But regardless, I guess we can have a look to also update that as over time I also came to the same conclusion.

Does API even survive sending a null inside the JSON array? This struct is not mean to be send to the API, but I see it actually is, so that's another issue on itself 😏

@mitar
Copy link
Contributor

mitar commented Dec 12, 2021

When do you plan to merge this?

@svanharmelen
Copy link
Member Author

Not sure yet... But certainly before the end of 2021, so in the coming few weeks.

@mitar
Copy link
Contributor

mitar commented Dec 15, 2021

Any reason why to delay? I am using this and it works well. I think it will be just hard to maintain this branch (it already has conflicts).

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.

Cannot use UpdateIssue to remove all issue assignees
2 participants