Change bindings version for every breaking change

[malashin]

@veeableful We should look into making the package more dependable in terms of versioning with all the breaking changes we do.

We can use http://labix.org/gopkg.in.

This is the link we have for go-sdl, but since all our tags major versions are zero there is only one version available: https://gopkg.in/veandco/go-sdl2.v0

Installation go get gopkg.in/veandco/go-sdl2.v0 Package import gopkg.in/veandco/go-sdl2.v0/sdl

Whenever a breaking change is committed we should increase the version via tags or a new branch.

The major version should be increased whenever the respective package API is being changed in an incompatible way.

Examples of modifications that DO NEED a major version change are: Removing or renaming any exposed name (function, method, type, etc) Adding, removing or renaming methods in an interface Adding a parameter to a function, method, or interface Changing the type of a parameter or result in a function, method, or interface Changing the number of results in a function, method, or interface Some struct changes (see details below)

On the other hand, the following modifications are FINE WITHOUT a major version change: Adding exposed names (function, method, type, etc) Renaming a parameter or result of a function, method, or interface Some struct changes (see details below)

If we are going to use gopkg.in then we need to decide if branches or tags are used for new versions.

1. Tags are less obstructive, since we are already using them and they are hidden in Releases. The problem is that tags are linked to a certain commit and it will be harder to maintain if we are going to update minor versions. Whenever non-breaking change is introduced we should increase minor version to 2.1, for example. When users update their package with go get gopkg.in/veandco/go-sdl2.v2 they will receive v2.1 in that case. And we should probably write what is actually changed in the release notes.
2. If branches are used: v1, v2 and so on. The latest one will always be up to date and will replace the master branch. But there will be a lot of branches in our case. Example: https://github.com/go-yaml/yaml/tree/v2

[veeableful]

My personal opinion is that we don't need to maintain old versions so I think I would go for tags. Perhaps we can start with version v1.0 and increment the minor version after some non-breaking updates. If we have breaking updates, then we'll increment the major version and continue from there (e.g. v1.1 -> v2.0).

I figure that should be simple enough to maintain and doesn't cause a lot of problems for the users. What do you think?

P.S. Do you think I should tag our current master as v1.0 now? :grimacing:

[malashin]

Ye, this is what I proposed in the Tags way.

Keep master as unstable. Add taged releases with every change (perhaps we should think about grouping several changes into one, but it seems not likely with the way development is going so far).

Add changelogs to each release like this: https://github.com/acemod/ACE3/releases https://github.com/Microsoft/vscode-go/releases

I would like all commit names to have issue number in them if they are done in response to an issue. Since there is not much stated in the commit messages, having link to an issue helps with finding what was changed and why.

sdl: events: Split JoyDeviceEvent into two different events #333
sdl: video: Make GL funcs into Window methods #325

Also having one CHAGELOG.md file in root dir with all the changes from version to version is helpful.

---

If we are going this way this is what needs to be done:

1. Add information about this in README.md, so people would actually use tagged releases instead of master.
2. Tag current commit or the one before the next breaking commit as v1.0. BREAKING.md moved into v1.0 release notes.
3. Add issue numbers to the commits done in response to the said issues. Update CONTRIBUTING.md.
4. Every next commit with reasonable changes (code or documentation), not github text files, tagged as minor version increase v1.1, v1.2... All releases have release notes with changes (I like how it's done in ACE3 https://github.com/acemod/ACE3/releases).
5. Every breaking commit increases the major version to v2.0, v3.0 and so on. Release notes.
6. With every new release CHANGELOG.md is updated. Example: https://www.libsdl.org/tmp/SDL/WhatsNew.txt

[veeableful]

Alright! I think once the existing PR is merged and the few recent issues we have are solved, I will tag master as v1.0, do some of the stuff you listed and we can start practicing our process then. Although I'm not sure if we should make a tagged release for every change. Perhaps we can tag it every month for starters?

As for the release note, I was wondering if we can just use git shortlog v0.2..master as the base release note, kind of like https://www.winehq.org/announce/3.7 to make the process simpler. We may need to help renaming PR commits that we merge to make them clearer when shown in the shortlog. After that, we can add "What's New" and "Bugs Fixed" stuff on top. Maybe we can label some PRs as "fixes" and "features" so we can find them easier when writing the release note.

Anyway, that's just my two cents. If you feel strongly about the other styles of release notes, I will support you :grimacing:

[malashin]

git log --oneline seems to be good for this. The problem is that first line of commit message might be too short.

Example of commit: sdl: events: Split JoyDeviceEvent into two different events #333

What it should be in the changelog imo: sdl: events: Split JoyDeviceEvent into JoyDeviceAddedEvent and JoyDeviceRemovedEvent #333

This is what I wrote in the BREAKING.md: Split JoyDeviceEvent into JoyDeviceAddedEvent and JoyDeviceRemovedEvent, since Which field can contain different types of data (int or JoystickID).

And it can get quite wordy to list all the changes. Name changes are really important and should not be hidden. There will be a huge list of them in our case, all done in one commit.

Also I think Wine's sdl/joystick: Commit message is better then our sdl: joystick: .... One character for path separator instead of two. ;)

Doing it every month is great for us, but forces others to use master branch if their issue is solved but not yet tagged.

[veeableful]

Hmm you have a point. How about tagging a minor version for every issue that is resolved then? I think we'll have a reasonable number of tags that way and every tag will have a purpose behind it (e.g. every new tag is a new issue fixed!).

About the commit message prefix, I personally think ours is easier to read and more consistent when read in many lines. But I did ask my colleague and he also seems to prefer the slash version so maybe we can think about changing to that :joy:

As for the release note, we can use git log --oneline or git shortlog to start with and add on the details such as breaking changes or new stuff on top for the "Breaking Changes" and "What's New" sections. We might need to start squashing the commits into a single change though so it makes more sense when it's read on the log :stuck_out_tongue:

Perhaps we should start writing these guidelines down somewhere, maybe in MAINTAINING.md file, and we keep revising it. After we're happy, we can start the v1.0 adventure!

[malashin]

I've looked in other alternatives and found that there will be and official Go solution to versioning, currently known as vgo.

https://github.com/golang/proposal/blob/master/design/24301-versioned-go.md https://github.com/golang/go/wiki/vgo https://research.swtch.com/vgo https://www.youtube.com/watch?v=F8nrpe0XWRg

$ vgo get rsc.io/quote@latest  # default
$ vgo get rsc.io/[email protected]
$ vgo get rsc.io/quote@'<v1.6' # finds v1.5.2

There will be some compatibility bits for vgo in Go 1.11 (August 2018), nothing major. Though Youtube video description states:

Go 1.11 will add opt-in support for package versions.

This twit states that vgo might be merged into 1.12: https://twitter.com/bradfitz/status/991674185612054529

The good thing is that they use tags for it to work. So what we've discussed earlier is compatible with vgo.

They propose the following package structures:

1. Have one branch with folders for each major version: go-sdl2, go-sdl2/v2 and so on. Each directory contains go.mod file with package version and its dependencies version.

module "my/thing"
require (
	"new/thing" v2.3.4
	"old/thing" v1.2.3
)
exclude "old/thing" v1.2.3
replace "bad/thing" v1.4.5 => "good/thing" v1.4.5

2. Have separate branches for each version: master, v2, v3 and so on. Each branch root dir with go.mod file.

Both solutions are good if old versions are kept alive with security or feature patches. In our case one master branch with version tags is enough.

We might need to add go.mod file later on, once this is a part Go.

There is also this:

Preparing New Versions (go release) We want to encourage authors to issue tagged releases of their modules, so we need to make that as easy as possible. We intend to add a go release command that can take care of as much of the bookkeeping as needed. For example, it might:

• Check for backwards-incompatible type changes, compared to the previous release. We run a check like this when working on the Go standard library, and it is very helpful.

• Suggest whether this release should be a new point release or a new minor release (because there's new API or because many lines of code have changed). Or perhaps always suggest a new minor release unless the author asks for a point release, to keep a potential go get -p useful.

• Scan all source files in the module, even ones that aren't normally built, to make sure that all imports can be satisfied by the requirements listed in go.mod. Referring back to the example in the download section, this check would make sure that logrus's go.mod lists x/sys.

As new best practices for releases arise, we can add them to go release so that authors always only have one step to check whether their module is ready for a new release.

[veeableful]

Nice find! So I guess this means we won't be required to use gopkg.in for versioning in the future. The go release command will also be quite handy to make sure we version correctly :smile:.

[veeableful]

I'm also thinking to tag our current version as v0.3 just so we have some breathing space as we work towards v1.0. Breaking changes since v0.2 are getting pretty long. Do you have any objection? :thinking:

[malashin]

I'm also thinking to tag our current version as v0.3 just so we have some breathing space as we work towards v1.0.

This won't hurt, go for it. There is official dep tool that can use them since go 1.6 and we are better off doing tagged releases once in a while.

[veeableful]

Cool! I pushed v0.3 to the repository now.

[malashin]

I'm currently working on CHANGELOG.md. Found this more or less popular standard Keep a Changelog.

Things to point out:

1. Rename all tags to adhere to Semantic Versioning. v0.3.0 instead of v0.3, v0.2.0 instead of v0.2 and so on.
2. Each version has a link to compare it to the previous one. Example https://github.com/veandco/go-sdl2/compare/v0.2...v0.3. Won't work with whats written lower because of the difference in the tag names.
3. Same info will be added to the release notes of each version.
4. I decided not to mention changes in the sdl examples.
5. Some changes are squashed into one line, if there were several issue links they are comma separated in the parenthesis:

• sdl/render: Add Renderer.GetLogicalSize(), Texture.UpdateYUV() (#264).

---

Changelog

All notable changes to this project will be documented in this file.

The format is based on Keep a Changelog and this project adheres to Semantic Versioning.

Unreleased

// TODO

0.3.0 - 2018-05-07

// TODO

0.2.0 - 2017-11-01

Added

• sdl/endian: Add BYTEORDER.
• sdl/hints: Add HintCallback type and AddHintCallback(), DelHintCallback() functions.
• sdl/keyboard: Add GetModState() (#230).
• sdl/pixels: Add PIXELFORMAT_RGBA32, PIXELFORMAT_ARGB32, PIXELFORMAT_BGRA32, PIXELFORMAT_ABGR32.
• sdl/render: Add Renderer.GetLogicalSize(), Texture.UpdateYUV() (#264).

Changed

• Rename packages from sdl_mixer, sdl_image, sdl_ttf to img, mix, ttf.
• gfx: Rename GFXPrimitiveSetFont() to SetFont().
• gfx: Rename GFXPrimitivesSetFontRotation() to SetFontRotation().
• mix/sdl_mixer: Change QuickLoad_WAV(mem unsafe.Pointer) (*Chunk, error) to QuickLoad_WAV(mem []byte) (*Chunk, error).
• sdl/joystick: Rename Update() to JoystickUpdate().
• sdl/render: Change Renderer.GetRendererInfo(info *RendererInfo) error to Renderer.GetInfo() (RendererInfo, error).
• sdl/render: Change Texture.Lock(rect *Rect, pixels *unsafe.Pointer, pitch *int) error to Texture.Lock(rect *Rect) ([]byte, int, error).
• sdl/render: Change Texture.Update(rect *Rect, pixels unsafe.Pointer, pitch int) error to Texture.Update(rect *Rect, pixels []byte, pitch int) error.
• sdl/render: Rename Renderer.GetRendererOutputSize() to Renderer.GetOutputSize().

Fixed

• gfx: Fix GetFramecount() to call SDL_getFramecount() instead of SDL_getFramerate (#258).
• gfx: Fix SetFont() segfault when passed a nil font data.
• gfx: Fix min() function used in many gfx calculations.

[0.1.0] - 2017-06-01

[veeableful]

@malashin Oh I didn't see this! It looks great! I just finished writing the release note but perhaps I should write a Fixed section as well.

[malashin]

Wow, you did a lot :) I like grouping of functions, structs and types. UpdateYUV() was a part of v0.2. It shows up in v0.3 most likely due to not squashed commits added, probably my first merged PR.

[veeableful]

You're right, I found the commit as the last one in v0.2. Gonna remove that now :stuck_out_tongue:. Thanks for the tip!

[malashin]

JoyDeviceEvent is now split into JoyDeviceAddedEvent and JoyDeviceRemovedEvent

This was done because JoyDeviceEvent's Which can contain two different types:

Sint32 | which | the joystick device index for the SDL_JOYDEVICEADDED event or the instance id for the SDL_JOYDEVICEREMOVED event

For C it's doesn't matter, but in Go we can't have two different types in one struct field. Convenience has nothing to do with the change, JoyDeviceEvent contained wrong type of Which on addition of joystick device.

[veeableful]

@malashin Gotcha! I've tweaked the description to be more correct.

Although on closer inspection, I wonder if any problem will be caused by having the type as int instead of int32? Specifically, I'm concerned about the extra 4 bytes because int in Go is 8 bytes for 64-bit machines according to my knowledge.

[malashin]

@veeableful

Although on closer inspection, I wonder if any problem will be caused by having the type as int instead of int32? Specifically, I'm concerned about the extra 4 bytes because int in Go is 8 bytes for 64-bit machines according to my knowledge.

Yes, this is the case. Go int is 8 bytes on 64-bit, C int is 4 bytes on 64-bit in most compilers (cgo included) for backwards compatibility. Running C.int(goint) will change the size from 8 to 4 bytes on 64-bit.

So far we've changed it to int32 in all the rendering stuff, but left it as int in places that return number of things, indexes or IDs. It more or less makes sense with how it is now and shouldn't cause any issues since all Go 64-bit ints are converted to 32-bit with C.int.

Having consistency with int types is nice, but not an easy decision if we should change them or not.

[malashin]

Though there is a problem of Go int becoming -1 if it doesn't fit into int32 after running int32(int) or C.int(int) on it.

[malashin]

I've just found this issue https://github.com/campoy/flappy-gopher/issues/9.

This means that gopkg.in won't really help with versioning, because optional packages are still importing original "github.com/veandco/go-sdl2/sdl" instead of "gopkg.in/veandco/go-sdl2.v0/sdl".

This makes it pretty much pointless.

[veeableful]

Yeah, that's true... I didn't think of that. I'll remove the note about it in README.md.