search

OoliteProject / oolite

The main Oolite repository.
https://www.oolite.space
544 stars 70 forks source link

add Ubuntu build #421

Closed HiranChaudhuri closed 1 year ago

HiranChaudhuri commented 1 year ago

We can now build on Ubuntu again. For changes on master a release is created, for changes on other branches a prerelease is created. This will allow to use the CI/CD pipeline for both releases and for development.

tsoj commented 1 year ago

What about removing the out-commented lines? Personally, when I read old code that I haven't written myself, I am often confused by commented out code, as I am not sure if it is actually code that is still important or not (except for stuff that got notes like "# this is for debug only; it creates huge logs and takes a long time to execute", where it is clear why it is commented out).

Otherwise I don't see anything wrong, and if it works, it works :D

AnotherCommander commented 1 year ago

Looking at the PR, it looks OK but I am not entirely sure about the pre-release part.

If I understand correctly the yml, when a push is made to master an actual full release will be created and any other push will result in a pre-release.. This means that 1.90 will no longer be listed as the latest official release, instead each nightly one will become the latest official. Essentially this means that a rolling release model is adopted. Am I getting it right and, if yes, is this what we want? Right now the nightlies are published as pre-releases and this preserves 1.90 as being the latest official build.

My preference would be to keep 1.90 as latest and maintain the nightlies originating from pushes to master as pre-releases. I think that the way it works now is fine, because the non-master pushes are being tested (i.e. compiled) anyway, it's just that no binaries are published for them. I think that the milestone releases should be the only ones that get the green "official release" tag.

HiranChaudhuri commented 1 year ago

What about removing the out-commented lines? Personally, when I read old code that I haven't written myself, I am often confused by commented out code, as I am not sure if it is actually code that is still important or not (except for stuff that got notes like "# this is for debug only; it creates huge logs and takes a long time to execute", where it is clear why it is commented out).

Otherwise I don't see anything wrong, and if it works, it works :D

Ok, I will look into either removing the commented out code or add a comment why it is still in there.

HiranChaudhuri commented 1 year ago

Looking at the PR, it looks OK but I am not entirely sure about the pre-release part.

If I understand correctly the yml, when a push is made to master an actual full release will be created and any other push will result in a pre-release.. This means that 1.90 will no longer be listed as the latest official release, instead each nightly one will become the latest official. Essentially this means that a rolling release model is adopted. Am I getting it right and, if yes, is this what we want? Right now the nightlies are published as pre-releases and this preserves 1.90 as being the latest official build.

My preference would be to keep 1.90 as latest and maintain the nightlies originating from pushes to master as pre-releases. I think that the way it works now is fine, because the non-master pushes are being tested (i.e. compiled) anyway, it's just that no binaries are published for them. I think that the milestone releases should be the only ones that get the green "official release" tag.

I agree the official releases shall be official releases, I still would like to see intermediate (or call them pre-releases) also for feature branches. that way it would be easier for others to see the effect of changes - especially when having to decide about a merge.

But as I am not a full master of Github Actions or the release system I would have to see the effects before I fully know if my workflow matches what I intended to. Is 'latest' really moved to prereleases as well? After all, prereleases are tagged with ${{ env.GITHUB_REF_NAME }}-${{ env.TODAY }}}}, which should result in the branch name plus the date.

Plus I believe there is room for improvement. For example, I am not yet 100% happy with the naming of the Linux build. And we might want to change the labeling, which is currently done on date but I just was not sure how the version numbers get counted in this project.

HiranChaudhuri commented 1 year ago

How did this get merged? Sorry, this was not my intention.