grossmj
commented
1 month ago The github workflow .github/workflows/ci_build.yml is building and testing for linux+mac+cygwin, so it's time to review the other stuff...
Yes, I was thinking about it when I saw you added building and testing for linux+mac+cygwin. Thanks for that by the way :+1:
The appveyor workflow should be removed?
We are already in the process of removing appveyor on other repos, so yes we should remove it for Dynamips as well.
The circleci workflow should be removed or turned into a github workflow?
Same for Circle CI, going forward we only plan to use Circle CI to build GNS3 itself as well as the GNS3 VMs.
One question though, should we send binaries for Windows + macOS on Sourceforge? If yes, I think doing it for new releases would be enough (no nightly builds). I can take care of this and remove the AppVeyor and Circle CI workflows.
snapcraft.yaml
I would like to keep this since we have plans to make snaps for GNS3 too.
If this is desired then I can try to make github workflows for it, otherwise it should be removed to avoid maintenance burdens.
That would be fantastic if you can give it a try :)
I was expecting the github workflows to run for pull requests but they need to be approved? It's fine to not run them automatically when the PR changes workflow files, but for regular PRs the workflows should run. Knowing if the changes compile is something that should be done before merging. The current settings are making you merge without checking that, which can lead to master having compile errors. This is not the default behavior so checking or changing things has be done by you or someone with access to the settings.
I am expecting the same since with these lines in the workflow:
pull_request:
branches: [ "master" ] # run for pull requests that target these branchesI am going to check why.
I'm not sure how dynamips releases are made. What can you tell me about it?
To create a new release:
- Update these files: https://github.com/GNS3/dynamips/commit/67eaa71ead4ca07c0771c6459394411988d3b90a
- Tag the repository:
git tag v0.2.22
git push origin v0.2.22That's it :)
The github workflow
.github/workflows/ci_build.ymlis building and testing for linux+mac+cygwin, so it's time to review the other stuff...appveyor.yml
Seems like a cygwin build that needs the libelf url updated and since winpcap is broken in chocolatey it also needs an alternative. The github workflow already does this (64-bit version, cygwin deprecated 32-bit). The appveyor workflow should be removed?
.circleci/config.yml
Seems like a nightly macos build that uploads the unstable version of dynamips to https://sourceforge.net/projects/gns-3/files/Nightly%20Builds/ . There are no executables there so it either stopped working and does not show the builds here, or it was disabled. The circleci workflow should be removed or turned into a github workflow?
snapcraft.yaml
Seems like a recipe for https://snapcraft.io/dynamips . I'm not familiar with it, but I tried to build with snapcraft and it complained about core18. If the intention is to keep this updated then it needs github workflows to automate builds. These channels seems appropriate:
For architectures, probably amd64 and x86, since they both have jit code. Maybe ppc too (jit code in unstable) if I ever manage to automate a build. If this is desired then I can try to make github workflows for it, otherwise it should be removed to avoid maintenance burdens.
Pull requests
I was expecting the github workflows to run for pull requests but they need to be approved? It's fine to not run them automatically when the PR changes workflow files, but for regular PRs the workflows should run. Knowing if the changes compile is something that should be done before merging. The current settings are making you merge without checking that, which can lead to master having compile errors. This is not the default behavior so checking or changing things has be done by you or someone with access to the settings.
Releases
I'm not sure how dynamips releases are made. What can you tell me about it?