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

[5.0] Move build system to github actions #1674

Draft
wants to merge 10 commits into
base: opentk5.0
Choose a base branch
from

Conversation

frederikja163
Copy link
Member

Purpose of this PR

  • Move the build system to github actions.
  • This allows us to be more mainstream and follow the rest of the industry. It allows us to remove a third party dependency on app veyor
  • Removes a lot of unnecessary files that weren't used
  • Cleans up the general project structure to make it more intuitive and easy to understand

Testing status

  • Tested on my local repo using normal github runners
  • Didn't test releasing on this project. But it works on some private projects and some projects from work, the script hasn't been modified from those, but we need to input a nuget token to fix it.

Comments

  • This wont run any tests as the tests require an implementation of opengl and glfw, this might be possible to add, but for now i didn't prioritize this.

@frederikja163 frederikja163 marked this pull request as draft October 22, 2023 17:37
@NogginBops
Copy link
Member

Cleans up the general project structure to make it more intuitive and easy to understand

This can be another discussion in another PR.
We've already had discussions about the namespaces before and we're not changing them, they are good as they are.

There is also a ton of formatting changes that we don't want to make (I'm guessing it's trying to fix 80 columns??).
We already have a coding convention that we are going to stick with, and if we want to change something we can discuss that as part of another issue or PR.

I also have opinions about creating automatic nuget packages using GitHub actions (it seems to be doing that when you push to main (not master which is what our main branch is called)).
I don't like it.
Having something auto-trigger just because you pushed something isn't a great workflow, you should be able to push things without worrying if some broken nuget package is going to be published.
It also doesn't create a good experience for the downstream users as they are getting bombarded by random nuget updates with no clear direction.

There are also some features that I have as "requirements" for a build system change:

  1. A way to manually build and release nuget packages (preferably build and publish locally to reduce the amount of magic in the build process). This is the better way to build packages.
  2. I want to have the version be defined only in RELEASE_NOTES.md and the nuget packages should get their version and change notes from there. (This I can compromise on slightly, but a "single source of truth" is highly desirable).

@frederikja163
Copy link
Member Author

The releases are still manual with this new github actions. Its just done through the github interface. We do a github release whenever there is a push to main and that push contains a version tag. This means you get change notes in the github repo and versioning from the github repo directly. Im still not done setting this up entirely, hence why this PR is markes as draft atm.

Atm, this PR also has a crazy amount of lines changed and im actively working on figuring out why that is. But i will redo the namespace renamings. My intent behind renaming namespaces was mostly to make it more internally consistent.

OpenTK.Graphics.OpenGL vs OpenTK.OpenAL which should probably be OpenTK.Audio.OpenAL instead to be more consistent. I was purely trying something by converting the namespaces. I will likely close this PR at some point and restart from a new branch.

@NogginBops
Copy link
Member

The namespace is already OpenTK.Audio.OpenAL, at least on the master branch.
The folders might not reflect this accurately though.

Having a release be published just because the commit message contains a magic string is exactly the type of magic I want to avoid in our build system. It will just be extra confusing, and is error prone. Or does this change it so that I have to use the GitHub web interface to create a GitHub release that will then trigger a publish to happen?

I would ideally see us not be dependent on the GitHub interface to be able to do releases.
A release should be something possible to create on any machine, and it shouldn't rely on some magic provided by GitHub.
If we could, a simple .ps1 script for the entire build would be desirable (or Makefile, etc, pick your poison).

I'm fine with having our CI be run on appveyor or GitHub but I would ideally remain as GitHub agnostic as possible.
OpenTK has changed hosting platform before and it's likely that at some (far) point in the future a similar transition is going to happen. I think it's worth considering this legacy.

@frederikja163
Copy link
Member Author

I will revisit the namespaces then when i get time to redo this PR.

Im saying we should use the github interface yes. Because the github interface can also auto generate release notes and other very valuable stuff.

Its not a magic string in a commit message. Its a tag you have to push, and its not some magic tag. The tag is the version you are creating. You can absolutely create releases from ci aswell, but then you wont get the auto-generated changelog, however we could likely add this to the github ci/cd aswell. IIRC the commands for releasing on your machine should be something like
git tag v5.0.0-pre.10; git push origin main -- tags --force And now users of the library can be more certain we have released the code we have on the repo, as the builds are all available directly from the github actions along with the logs. Its going to be easier to release with the new system not harder.

In terms of appveyor vs github, we would for sure have the same problem with appveyor no? If for some reason appveyor is not supported in the future we would have to change from appveyor anyways. Github actions at this point is way bigger than appveyor, so i see it as more risky to be on appveyor than github. Furthermore there are lots of tools for converting build scripts from github to gitlab build scripts aswell. Im sure with how big and universal github actions are at this point, if a better alternative shows up there are going to be tools to swap from one to the other.

@NogginBops
Copy link
Member

I'll take some time to more carefully consider these changes.

Though I will say that automatic change notes are generally quite bad for consumers. Git messages are often oriented towards the developers of the library and not the developers using the library. While change notes should be consumer oriented.
Writing change notes generally doesn't take me more than a few minutes so that isn't really something that needs to be more efficient.

... and other very valuable stuff.

What other features exist that are useful?

@frederikja163
Copy link
Member Author

You can for example automatically create discussion threads for a new release. Im not sure if this is something we want, but it is possible. It also means we can make releases from our phones and generally manage everything whithout having a computer handy.
image

Change notes are created by the pull requests closed not the commits. So its the same as what you are doing now, except at the click of a button which also goes a long way to minimize potential errors. You do get a chance to change the release notes before releasing aswell.

I noticed we also currently dont tag our releases with vx.x.x but i can easily modify the actions script to work with our current way of naming release tags.

Just to quickly summarize, the primary reasons i want to switch to github actions:

  • Its more standard and we are quite late in the move to github actions
  • Its a good time to clean up some other ancient artifacts in the repo (a clean desk makes a clean mind)
    With this im talking about stuff like the stylecops, inconsistencies in folder structure etc.
    Which will also make it less daunting for new contributers.
  • Github actions is (in my opinion atleast) a lot simpler than appveyor, specifically because it doesn't require you to go to a different site for building.
  • We can more easily provide a gurante that the commit linked in the release notes match the binary files on nuget and github, as it hasn't been generated on a personal computer but instead through a cloud service with publicly available logs.

@NogginBops
Copy link
Member

So its the same as what you are doing now

I'm not copying the PR names into the release notes, tried it for a release and it was not very usable. But I get the point.

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.

None yet

2 participants