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

codecs: more docs, a terminology guide, consistency in options. #221

Merged
merged 1 commit into from
Aug 12, 2021

Conversation

warpfork
Copy link
Collaborator

No breaking changes. In fact I think this should be behaviorally identical to before, despite the size of the diff. There are some functions that are explicitly marked as deprecated now, but I am not removing them in this diff.

One exception: some struct names did change. However, these have not been included in any tagged release so far, so I believe changing them now is still pretty fair game, and the number of affected people should be slim to none.

The main source of changes is pushing towards nomenclature consistency, which is described in the new readme file also included the diff.

@warpfork warpfork requested review from rvagg and mvdan August 12, 2021 14:20
@warpfork warpfork force-pushed the codec-terminology-and-config-consistency branch from 33d36a6 to 09ad1b7 Compare August 12, 2021 14:25
No breaking changes.  In fact I think this should be behaviorally
identical to before, despite the size of the diff.
There are some functions that are explicitly marked as deprecated now,
but I am not removing them in this diff.

One exception: some struct names did change.  However, these have not
been included in any tagged release so far, so I believe changing them
now is still pretty fair game, and the number of affected people should
be slim to none.

The main source of changes is pushing towards nomenclature consistency,
which is described in the new readme file also included the diff.
@warpfork warpfork force-pushed the codec-terminology-and-config-consistency branch from 09ad1b7 to 6d3da1f Compare August 12, 2021 14:27
@warpfork
Copy link
Collaborator Author

warpfork commented Aug 12, 2021

It was also discussed in another forum earlier that perhaps it would be nice if the zero values for these config options would actually be the default for that codec. However, that would conflict with using the same constants to describe things across packages, which seems silly, so I've dropped zero-coincides-with-the-default as a goal.

@mvdan
Copy link
Contributor

mvdan commented Aug 12, 2021

Since the default options are not the zero value, we might want to add a func DefaultOptions() OptionsType later on - but not a blocker.

@warpfork warpfork merged commit 1285c1d into master Aug 12, 2021
@warpfork warpfork deleted the codec-terminology-and-config-consistency branch August 12, 2021 15:02
@aschmahmann aschmahmann mentioned this pull request Aug 23, 2021
62 tasks
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.

2 participants